refactor(router): clean up internal hooks (#46321)

* beforePreactivation hook is unused
* The only place that uses afterPreactivation does not use the arguments

Not to say we won't want to provide hooks similar to this in the future,
but the current state is over-engineered for what it's being used for.

PR Close #46321

goldens/size-tracking/integration-payloads.json

@@ -33,7 +33,7 @@
   "cli-hello-world-lazy": {
     "uncompressed": {
       "runtime": 2835,
-      "main": 237828,
+      "main": 237084,
       "polyfills": 33842,
       "src_app_lazy_lazy_module_ts": 780
     }

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -998,9 +998,6 @@
   {
     "name": "defaultMalformedUriErrorHandler"
   },
-  {
-    "name": "defaultRouterHook"
-  },
   {
     "name": "defaultScheduler"
   },

packages/router/src/router.ts

@@ -330,30 +330,6 @@ export interface NavigationTransition {
   guardsResult: boolean|UrlTree|null;
 }
 
-/**
- * @internal
- */
-export type RouterHook = (snapshot: RouterStateSnapshot, runExtras: {
-  appliedUrlTree: UrlTree,
-  rawUrlTree: UrlTree,
-  skipLocationChange: boolean,
-  replaceUrl: boolean,
-  navigationId: number
-}) => Observable<void>;
-
-/**
- * @internal
- */
-function defaultRouterHook(snapshot: RouterStateSnapshot, runExtras: {
-  appliedUrlTree: UrlTree,
-  rawUrlTree: UrlTree,
-  skipLocationChange: boolean,
-  replaceUrl: boolean,
-  navigationId: number
-}): Observable<void> {
-  return of(null) as any;
-}
-
 /**
  * The equivalent `IsActiveMatchOptions` options for `Router.isActive` is called with `true`
  * (exact = true).
@@ -490,16 +466,12 @@ export class Router {
   private lastSuccessfulId: number = -1;
 
   /**
-   * Hooks that enable you to pause navigation,
-   * either before or after the preactivation phase.
+   * Hook that enables you to pause navigation after the preactivation phase.
    * Used by `RouterModule`.
    *
    * @internal
    */
-  hooks: {
-    beforePreactivation: RouterHook,
-    afterPreactivation: RouterHook
-  } = {beforePreactivation: defaultRouterHook, afterPreactivation: defaultRouterHook};
+  afterPreactivation: () => Observable<void> = () => of(void 0);
 
   /**
    * A strategy for extracting and merging URLs.
@@ -771,24 +743,6 @@ export class Router {
                        }
                      }),
 
-                     // Before Preactivation
-                     switchTap(t => {
-                       const {
-                         targetSnapshot,
-                         id: navigationId,
-                         extractedUrl: appliedUrlTree,
-                         rawUrl: rawUrlTree,
-                         extras: {skipLocationChange, replaceUrl}
-                       } = t;
-                       return this.hooks.beforePreactivation(targetSnapshot!, {
-                         navigationId,
-                         appliedUrlTree,
-                         rawUrlTree,
-                         skipLocationChange: !!skipLocationChange,
-                         replaceUrl: !!replaceUrl,
-                       });
-                     }),
-
                      // --- GUARDS ---
                      tap(t => {
                        const guardsStart = new GuardsCheckStart(
@@ -866,23 +820,7 @@ export class Router {
                        return undefined;
                      }),
 
-                     // --- AFTER PREACTIVATION ---
-                     switchTap((t: NavigationTransition) => {
-                       const {
-                         targetSnapshot,
-                         id: navigationId,
-                         extractedUrl: appliedUrlTree,
-                         rawUrl: rawUrlTree,
-                         extras: {skipLocationChange, replaceUrl}
-                       } = t;
-                       return this.hooks.afterPreactivation(targetSnapshot!, {
-                         navigationId,
-                         appliedUrlTree,
-                         rawUrlTree,
-                         skipLocationChange: !!skipLocationChange,
-                         replaceUrl: !!replaceUrl,
-                       });
-                     }),
+                     switchTap(() => this.afterPreactivation()),
 
                      // --- LOAD COMPONENTS ---
                      switchTap((t: NavigationTransition) => {

packages/router/src/router_module.ts

@@ -558,7 +558,7 @@ export class RouterInitializer implements OnDestroy {
         router.setUpLocationChangeListener();
         resolve(true);
       } else if (opts.initialNavigation === 'enabledBlocking') {
-        router.hooks.afterPreactivation = () => {
+        router.afterPreactivation = () => {
           // only the initial navigation should be delayed
           if (!this.initNavigation) {
             this.initNavigation = true;
@@ -567,7 +567,7 @@ export class RouterInitializer implements OnDestroy {
 
             // subsequent navigations should not be delayed
           } else {
-            return of(null) as any;
+            return of(void 0);
           }
         };
         router.initialNavigation();
@@ -598,7 +598,7 @@ export class RouterInitializer implements OnDestroy {
     preloader.setUpPreloading();
     routerScroller.init();
     router.resetRootComponentType(ref.componentTypes[0]);
-    this.resultOfPreactivationDone.next(null!);
+    this.resultOfPreactivationDone.next(void 0);
     this.resultOfPreactivationDone.complete();
   }
 

packages/router/test/integration.spec.ts

@@ -875,7 +875,7 @@ describe('Integration', () => {
     });
 
 
-    it('should eagerly update the URL with urlUpdateStrategy="eagar"',
+    it('should eagerly update the URL with urlUpdateStrategy="eager"',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = TestBed.createComponent(RootCmp);
          advance(fixture);
@@ -889,18 +889,21 @@ describe('Integration', () => {
          expect(fixture.nativeElement).toHaveText('team 22 [ , right:  ]');
 
          router.urlUpdateStrategy = 'eager';
-         (router as any).hooks.beforePreactivation = () => {
+         router.events.subscribe(e => {
+           if (!(e instanceof GuardsCheckStart)) {
+             return;
+           }
            expect(location.path()).toEqual('/team/33');
            expect(fixture.nativeElement).toHaveText('team 22 [ , right:  ]');
            return of(null);
-         };
+         });
          router.navigateByUrl('/team/33');
 
          advance(fixture);
          expect(fixture.nativeElement).toHaveText('team 33 [ , right:  ]');
        })));
 
-    it('should eagerly update the URL with urlUpdateStrategy="eagar"',
+    it('should eagerly update the URL with urlUpdateStrategy="eager"',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = TestBed.createComponent(RootCmp);
          advance(fixture);
@@ -930,7 +933,7 @@ describe('Integration', () => {
          expect(location.path()).toEqual('/login');
        })));
 
-    it('should set browserUrlTree with urlUpdateStrategy="eagar" and false `shouldProcessUrl`',
+    it('should set browserUrlTree with urlUpdateStrategy="eager" and false `shouldProcessUrl`',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = TestBed.createComponent(RootCmp);
          advance(fixture);
@@ -957,7 +960,7 @@ describe('Integration', () => {
          expect((router as any).browserUrlTree.toString()).toBe('/team/22');
        })));
 
-    it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eagar"',
+    it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eager"',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = TestBed.createComponent(RootCmp);
          advance(fixture);
@@ -991,7 +994,7 @@ describe('Integration', () => {
          expect(fixture.nativeElement).toHaveText('team 33 [ , right:  ]');
        })));
 
-    it('should set `state` with urlUpdateStrategy="eagar"',
+    it('should set `state` with urlUpdateStrategy="eager"',
        fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => {
          router.urlUpdateStrategy = 'eager';
          router.resetConfig([