PR 10823 and 10824 review

Both pull requests implement the same Angular 21 router and hydration migration from the same base commit. Both build and run. PR 10824 reaches the same result with less code and cleaner dependency boundaries.

Recommendation: approve and merge PR 10824. Close PR 10823 as superseded. Do not merge both.

reviewed 2026-09-22 base 0accd16451 10823 head 8d4761ac72 10824 head 7c66c9e741

Findings

clear

No merge-blocking defect found

The router provider, hydration provider, scroll behavior, custom URL serializer, and eleven migrated guards compile and start correctly in both PRs. Browser hydration completed without an Angular console error.

decision

The PRs are alternatives

Both branches start at 0accd16451, touch the same 23 files, and solve the same task. Merging both would add conflict resolution with no product benefit.

preferred

PR 10824 is the smaller implementation

PR 10824 removes 95 net lines. PR 10823 removes one net line. PR 10824 avoids three private guard-state services and moves the unsaved-changes prompt to a shared confirmation constant, so the common guard no longer imports a messages feature constant.

test gap

Guard behavior has no focused regression suite

The repository has no specs for the migrated authentication, permission, creator-approval, white-label owner, or can-deactivate guards. The migration preserves their branches by inspection, but authenticated role combinations still need a short QA pass. No generated white-label project exists in this checkout, so a tenant build was not available.

PR comparison

PR 10823

Valid, but superseded

  • Uses explicit CanActivateFn and CanDeactivateFn types.
  • Keeps mutable guard state in root-injected helper services.
  • Adds 614 lines and removes 615.
  • Passes the same build and runtime checks as PR 10824.

PR 10824

Preferred for merge

  • Uses direct functional guards with less supporting code.
  • Keeps router features together in appRouterProviders.
  • Adds 531 lines and removes 626.
  • Moves the confirmation text to shared/constants.

Validation evidence

Check PR 10823 PR 10824 Notes
git diff --check Pass Pass No whitespace errors.
QA browser and SSR build Pass Pass Angular 21 application build completed in about 78 seconds per PR.
SSR smoke test Pass Pass / and /privacy-policy returned HTTP 200 with hydration markers.
Browser hydration Pass Pass No Angular router, injector, or hydration errors.
Migration completeness Pass Pass No old class-guard references or root RouterModule.forRoot calls remain.
Generated white-label build Not run Not run angular.json contains only the TopFans project in this checkout.

The QA build still reports the existing 12.30 MB initial-bundle warning and existing CommonJS warnings. Neither PR introduces a new build failure.

Post-merge QA

  1. Open a public SSR route and confirm the browser hydrates without replacing the server-rendered page.
  2. Check unauthenticated login, authenticated login redirect, social callback, and saved return URL flows.
  3. Check fan, pending creator, approved creator, delegated user, white-label owner, and white-label non-owner routes.
  4. Leave Messages with unsaved changes and verify both choices in the native confirmation prompt.
  5. Build one generated white-label project before release.