All notes

PR 10839 review-astra

Edit-preference and the white-label cover page stop loading SharedModule.

Reviewed 24 September 202682 files+873 / -446 lines

PR 10839 · Base 9897f212fd · Head e92a285200

Should I merge it now? Yes.

I found no blocking regression in the reviewed changes.

The route definitions retain their paths, guards and data. Component logic stays the same. The compiled templates retain their used dependencies, and both migrated entries stop loading SharedModule. The smaller bundles are measurable.

This recommendation applies to e92a285200. It is a code and build review. The logged-in browser checks below are the author's evidence, not checks I reran.

What changes

  • EditPreferenceModule and its routing module become EDIT_PREFERENCE_ROUTES. Its 48 declared components become standalone.
  • WhitelabelCoverPageModule and its routing module become WHITELABEL_COVER_PAGE_ROUTES. Its 25 declared components become standalone.
  • VipOfferShareModalComponent and ScrollContainerLoaderDirective become standalone too. SharedModule imports and still exports both.
  • Dashboard is the only remaining lazy application entry that loads SharedModule, down from three.

What I checked

Routes and component behavior

An AST comparison found 74 of the 82 changed files differ only in imports, standalone metadata or comments. The other eight contain route and module wiring, the DatePipe provider, or the removed HTML binding. No component method body changes.

The cover-page route array is unchanged after removing the NgModule wrapper and renaming the export. Edit-preference retains its route definitions; its nested cover-page lazy import now points at the route array. The app's parent guards and permission data remain in place.

Template dependencies

Fresh Angular compilation emits all 650 components present in the base output. Of those, 644 retain exactly the same matched directive and pipe dependencies. The other six lose only NativeElementInjectorDirective from ngx-intl-tel-input. None of those six templates contains a phone input.

The six are GroupBadgesComponent, BlockUsersComponent, PreviewCreatorBioComponent, VipOfferShareModalComponent, AddMarketingPageSectionComponent and AddSectionLayoutPickerModalComponent.

Providers remain available

DelegateDetailsComponent now provides DatePipe, and its transaction-detail modal remains a child in its template. DatePipe is also already in the shared root provider list registered by AppModule. The provider move does not leave either component without it.

AppModule provides TitleCasePipe, and NgxImageCompressService declares providedIn: 'root'. Removing their EditPreferenceModule registrations does not remove access to them. SharedModule's provider list and all 160 exports are unchanged.

The removed social binding was invalid

The PR removes [socialHandles]="socialHandles" from the social-links modal. That component has no socialHandles input. Its existing subscription reads saved handles from LinkInBioDataService, so this deletion does not remove the actual data path. The old CUSTOM_ELEMENTS_SCHEMA had hidden the invalid binding.

One minor cleanup

git diff --check reports trailing whitespace at line 8 of pending-request-popup.component.ts. Remove it when convenient. It is not a reason to hold the merge.

Independent bundle measurements

Lazy entryBase MBPR MBReduction
Edit-preference6.0334.32428.3%
White-label cover page5.2903.18339.8%
Dashboard5.6575.6520.09%
Uncompressed JavaScript added by each entry after the initial load, in decimal MB. Each row includes its static dependencies. Do not add the rows together.

Initial JavaScript falls by 4 bytes, from 7,140,936 to 7,140,932 bytes. SharedModule appears in the static dependency graph of 1 of 45 lazy application entries. Those entries include dynamically loaded components as well as routes.

I calculated these values from Angular's build statistics, followed static JavaScript imports and subtracted the initial main, polyfills and scripts chunks. These are bundle sizes, not compressed transfer sizes or loading times.

Checks run for this review

CheckResult
TopFans QA client and server buildPass. Fresh build at the PR head. 42 warning groups, matching the saved base build's count.
Angular compilerPass. Fresh compilation and a comparison of all 650 component dependency lists against the saved base output.
Generated ajwhite QA buildPass. Generated ajwhite from QA API data in the isolated worktree and compiled its client and server bundles.
Source reviewPass. Route comparison, component AST comparison, provider inspection and shared export comparison.
Whitespace checkOne minor issue. Trailing whitespace in pending-request-popup.component.ts.

Builds use Node's 10 GB heap limit and disable prerendering. The head was checked in an isolated worktree. Base measurements and compiled output come from the preceding review of 9897f212fd.

GitHub reports the PR open, mergeable and clean. No status checks were listed at review time. The target branch tip was c206a6662c; these checks cover the PR head against its merge base, not a fresh build of the merge result.

Coverage limits

I did not rerun logged-in application flows, SSR response comparisons, hydration, development builds or the full prerender list. Compilation checks template wiring; it does not establish that every form and modal works in a signed-in session.

The author reports matching server rendering across 138 URLs, logged-in checks across 34 edit-preference and cover-page URLs, all 47 add-section layouts, header and footer editors, previews, sub-pages, the VIP picker, saved social handles and color-picker coordination. The author also reports successful development builds. Treat those as author-reported results.

The author could not open delegate details because the test account had no delegates. The provider wiring checks out in source, so I would not block this refactor on that gap. Keep a delegate-details and transaction-modal smoke test in the release checklist when a suitable account is available.

Review references