All notes

PR 10840 review-astra

Dashboard becomes standalone. SharedModule and MarketingPageSharedModule are deleted.

Reviewed 24 September 202693 files+1,423 / -1,184 lines

PR 10840 · Base e92a285200 · Head 7c0b9351ac

Should I merge it now? Yes.

I found no blocking regression at the reviewed commit.

The dashboard routes retain their paths, guards and data. The route keeps its own DashboardModuleService instance. Fresh TopFans and generated ajwhite QA builds pass, and compiled template dependencies retain the components and pipes the templates use.

This is a code and build recommendation for 7c0b9351ac. I did not rerun signed-in browser flows or complete a checkout.

What changes

  • The remaining SharedModule declarations become standalone, including checkout components, video players, dialogs and pipes.
  • DashboardModule and its routing module become DASHBOARD_ROUTES. Its 36 declared components become standalone.
  • Marketing, comparison, marketing pro and the cover-page section editor import their template dependencies directly. SharedModule and MarketingPageSharedModule are removed.
  • No lazy application entry loads SharedModule, down from one of 45 entries.

What I checked

Routes and behavior stay the same

An AST comparison found that 87 of 93 changed files differ only in imports, standalone metadata or comments. The other six cover route wiring, three deleted modules and the removed video binding. No component method body changes.

The dashboard route array is identical after removing the new providers and accounting for its exported name. The parent app route remains lazy. See the dashboard route and provider list.

Dashboard keeps its stateful providers

The top-level dashboard route provides all five services and pipes previously supplied by DashboardModule. It also provides isPostCollaboratorPipe, which injects MyProfile and must use the dashboard's DashboardModuleService instance.

The shared root provider registrations are unchanged. They retain the dependencies used by root-created modals and the existing multiselect and circle-progress configuration. See shared root providers.

Compiled templates retain their dependencies

Fresh Angular compilation emits 646 components, compared with 650 in the saved base compilation. After normalizing import aliases, 619 have identical dependency lists. Another 26 only lose ngx-intl-tel-input's NativeElementInjectorDirective. I checked that none of those 26 templates contains a phone input.

ForumPostDetailComponent resolves NgbCarousel and NgbSlide through the package's main export instead of its carousel entry. The installed package re-exports the same classes.

The four components absent from the new output are ProfileDetailComponent, SubscriptionSelectModalComponent, SubscriptionPlanDurationModalComponent and the old typeahead SearchBarComponent. Their active import chain ended at the removed modules. The typeahead's remaining template use is inside an HTML comment.

Video cover selection still has its data path

The deleted coverImage binding targeted an input that VideoPlayerV3Component does not declare. The old CUSTOM_ELEMENTS_SCHEMA hid it. VideoTrimComponent still calls the player's setCoverImage method when a cover is selected. See the existing cover-selection method.

Minor cleanup, no merge blocker

The whitespace check finds trailing spaces in merchandise-purchase.component.ts at line 6, voice-circle.component.ts at line 45, login-dialog.component.ts at line 11 and subscription-end-modal.component.ts at line 16. Remove them when convenient.

Independent bundle measurements

Lazy entryBase MBPR MBReduction
Dashboard5.6525.0979.8%
Marketing2.1611.22643.3%
Comparison2.1621.22643.3%
Marketing pro2.1942.0655.9%
Uncompressed JavaScript added after the initial load, in decimal MB. Each entry includes its static dependencies. The rows overlap and must not be added together.

Initial JavaScript drops from 7,140,932 to 7,126,464 bytes, a reduction of 14,468 bytes. None of the 45 lazy application entries has SharedModule in its static dependency graph.

I calculated these values from the fresh QA build's statistics and the saved base QA build at e92a285200. These are bundle sizes, not compressed transfer sizes or measured loading times.

Checks run for this review

CheckResult
TopFans QA client and server buildPass. 42 warning groups, matching the base.
Generated ajwhite QA client and server buildPass. Generated from QA API data. 42 warning groups.
Angular compilationPass. Compared all emitted component dependency lists with the saved base output.
Source comparisonsPass. Component ASTs, route array, providers, removed modules and video-cover wiring.
Whitespace checkFour trailing-whitespace lines. No functional impact.

Builds ran in an isolated checkout with a 10 GB Node heap limit and prerendering disabled. Base artifacts come from the preceding review of PR 10839. GitHub reports this PR open, mergeable and clean, with no status checks listed.

The target branch tip was 1cebc9ab97. This review compares the PR head with its merge base, e92a285200. I did not build a fresh merge result.

Runtime coverage still has limits

I did not rerun signed-in flows, SSR response comparisons, hydration, development builds or the full prerender list. A successful compilation checks template wiring but cannot establish that every payment and dialog interaction works.

The author reports QA and development builds for all four commits, SSR comparisons over 138 URLs for the first three and 145 for the last, signed-in checks over 30 dashboard URLs, all post-type tabs, feed dialogs and all 47 cover-page layouts. Those are author-reported results, separate from the checks above.

The author did not complete purchases, posting, saving, order confirmation or deletion. Keep checkout, order confirmation and deletion in the release smoke-test checklist. I found no specific regression in their changed wiring that warrants holding this refactor.

Review references