fix(router): adjust ChildActivation events to only fire when the child is actually changing (#19043)

* The problem was with the `fireChildActivationStart` function. It was taking a `path` param, which was an
array of `ActivatedRouteSnapshot`s. The function was being fired for each piece of the route that was being
activated. This resulted in far too many `ChildActivationStart` events being fired, and being fired on routes
that weren't actually getting activated. This change fires the event only for those routes that are actually
being activated.

fixes #18942

PR Close #19043

packages/router/src/events.ts

@@ -7,7 +7,8 @@
  */
 
 import {Route} from './config';
-import {RouterStateSnapshot} from './router_state';
+import {ActivatedRouteSnapshot, RouterStateSnapshot} from './router_state';
+
 
 /**
  * @whatItDoes Base for events the Router goes through, as opposed to events tied to a specific
@@ -264,8 +265,11 @@ export class RouteConfigLoadEnd {
 export class ChildActivationStart {
   constructor(
       /** @docsNotRequired */
-      public route: Route) {}
+      public snapshot: ActivatedRouteSnapshot) {}
-  toString(): string { return `ChildActivationStart(path: '${this.route.path}')`; }
+  toString(): string {
+    const path = this.snapshot.routeConfig && this.snapshot.routeConfig.path || '';
+    return `ChildActivationStart(path: '${path}')`;
+  }
 }
 
 /**
@@ -277,8 +281,11 @@ export class ChildActivationStart {
 export class ChildActivationEnd {
   constructor(
       /** @docsNotRequired */
-      public route: Route) {}
+      public snapshot: ActivatedRouteSnapshot) {}
-  toString(): string { return `ChildActivationEnd(path: '${this.route.path}')`; }
+  toString(): string {
+    const path = this.snapshot.routeConfig && this.snapshot.routeConfig.path || '';
+    return `ChildActivationEnd(path: '${path}')`;
+  }
 }
 
 /**

packages/router/src/pre_activation.ts

@@ -202,8 +202,8 @@ export class PreActivation {
     const checks$ = from(this.canActivateChecks);
     const runningChecks$ = concatMap.call(
         checks$, (check: CanActivate) => andObservables(from([
-                   this.fireChildActivationStart(check.path), this.runCanActivateChild(check.path),
+                   this.fireChildActivationStart(check.route.parent),
-                   this.runCanActivate(check.route)
+                   this.runCanActivateChild(check.path), this.runCanActivate(check.route)
                  ])));
     return every.call(runningChecks$, (result: boolean) => result === true);
     // this.fireChildActivationStart(check.path),
@@ -217,16 +217,11 @@ export class PreActivation {
    * return
    * `true` so checks continue to run.
    */
-  private fireChildActivationStart(path: ActivatedRouteSnapshot[]): Observable<boolean> {
+  private fireChildActivationStart(snapshot: ActivatedRouteSnapshot|null): Observable<boolean> {
-    if (!this.forwardEvent) return of (true);
+    if (snapshot !== null && this.forwardEvent) {
-    const childActivations = path.slice(0, path.length - 1).reverse().filter(_ => _ !== null);
+      this.forwardEvent(new ChildActivationStart(snapshot));
-
+    }
-    return andObservables(map.call(from(childActivations), (snapshot: ActivatedRouteSnapshot) => {
+    return of (true);
-      if (this.forwardEvent && snapshot._routeConfig) {
-        this.forwardEvent(new ChildActivationStart(snapshot._routeConfig));
-      }
-      return of (true);
-    }));
   }
   private runCanActivate(future: ActivatedRouteSnapshot): Observable<boolean> {
     const canActivate = future._routeConfig ? future._routeConfig.canActivate : null;

packages/router/src/router.ts

@@ -21,7 +21,7 @@ import {applyRedirects} from './apply_redirects';
 import {LoadedRouterConfig, QueryParamsHandling, Route, Routes, validateConfig} from './config';
 import {createRouterState} from './create_router_state';
 import {createUrlTree} from './create_url_tree';
-import {ChildActivationEnd, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouterEvent, RoutesRecognized} from './events';
+import {ChildActivationEnd, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RoutesRecognized} from './events';
 import {PreActivation} from './pre_activation';
 import {recognize} from './recognize';
 import {DefaultRouteReuseStrategy, DetachedRouteHandleInternal, RouteReuseStrategy} from './route_reuse_strategy';
@@ -864,8 +864,8 @@ class ActivateRoutes {
     const children: {[outlet: string]: any} = nodeChildrenAsMap(currNode);
     futureNode.children.forEach(
         c => { this.activateRoutes(c, children[c.value.outlet], contexts); });
-    if (futureNode.children.length && futureNode.value.routeConfig) {
+    if (futureNode.children.length) {
-      this.forwardEvent(new ChildActivationEnd(futureNode.value.routeConfig));
+      this.forwardEvent(new ChildActivationEnd(futureNode.value.snapshot));
     }
   }
 

packages/router/test/bootstrap.spec.ts

@@ -83,8 +83,9 @@ describe('bootstrap', () => {
       const data = router.routerState.snapshot.root.firstChild !.data;
       expect(data['test']).toEqual('test-data');
       expect(log).toEqual([
-        'TestModule', 'NavigationStart', 'RoutesRecognized', 'GuardsCheckStart', 'GuardsCheckEnd',
+        'TestModule', 'NavigationStart', 'RoutesRecognized', 'GuardsCheckStart',
-        'ResolveStart', 'ResolveEnd', 'RootCmp', 'NavigationEnd'
+        'ChildActivationStart', 'GuardsCheckEnd', 'ResolveStart', 'ResolveEnd', 'RootCmp',
+        'ChildActivationEnd', 'NavigationEnd'
       ]);
       done();
     });
@@ -121,7 +122,7 @@ describe('bootstrap', () => {
          // ResolveEnd has not been emitted yet because bootstrap returned too early
          expect(log).toEqual([
            'TestModule', 'RootCmp', 'NavigationStart', 'RoutesRecognized', 'GuardsCheckStart',
-           'GuardsCheckEnd', 'ResolveStart'
+           'ChildActivationStart', 'GuardsCheckEnd', 'ResolveStart'
          ]);
 
          router.events.subscribe((e) => {

packages/router/test/integration.spec.ts

@@ -705,15 +705,18 @@ describe('Integration', () => {
        expect(user.recordedParams).toEqual([{name: 'init'}, {name: 'fedor'}]);
 
        expectEvents(recordedEvents, [
-         [NavigationStart, '/user/init'], [RoutesRecognized, '/user/init'],
+         [NavigationStart, '/user/init'],   [RoutesRecognized, '/user/init'],
-         [GuardsCheckStart, '/user/init'], [GuardsCheckEnd, '/user/init'],
+         [GuardsCheckStart, '/user/init'],  [ChildActivationStart],
-         [ResolveStart, '/user/init'], [ResolveEnd, '/user/init'], [NavigationEnd, '/user/init'],
+         [GuardsCheckEnd, '/user/init'],    [ResolveStart, '/user/init'],
+         [ResolveEnd, '/user/init'],        [ChildActivationEnd],
+         [NavigationEnd, '/user/init'],
 
          [NavigationStart, '/user/victor'], [NavigationCancel, '/user/victor'],
 
-         [NavigationStart, '/user/fedor'], [RoutesRecognized, '/user/fedor'],
+         [NavigationStart, '/user/fedor'],  [RoutesRecognized, '/user/fedor'],
-         [GuardsCheckStart, '/user/fedor'], [GuardsCheckEnd, '/user/fedor'],
+         [GuardsCheckStart, '/user/fedor'], [ChildActivationStart],
-         [ResolveStart, '/user/fedor'], [ResolveEnd, '/user/fedor'],
+         [GuardsCheckEnd, '/user/fedor'],   [ResolveStart, '/user/fedor'],
+         [ResolveEnd, '/user/fedor'],       [ChildActivationEnd],
          [NavigationEnd, '/user/fedor']
        ]);
      })));
@@ -740,8 +743,8 @@ describe('Integration', () => {
          [NavigationStart, '/invalid'], [NavigationError, '/invalid'],
 
          [NavigationStart, '/user/fedor'], [RoutesRecognized, '/user/fedor'],
-         [GuardsCheckStart, '/user/fedor'], [GuardsCheckEnd, '/user/fedor'],
+         [GuardsCheckStart, '/user/fedor'], [ChildActivationStart], [GuardsCheckEnd, '/user/fedor'],
-         [ResolveStart, '/user/fedor'], [ResolveEnd, '/user/fedor'],
+         [ResolveStart, '/user/fedor'], [ResolveEnd, '/user/fedor'], [ChildActivationEnd],
          [NavigationEnd, '/user/fedor']
        ]);
      })));
@@ -1463,10 +1466,10 @@ describe('Integration', () => {
              expect(location.path()).toEqual('/');
              expectEvents(recordedEvents, [
                [NavigationStart, '/team/22'], [RoutesRecognized, '/team/22'],
-               [GuardsCheckStart, '/team/22'], [GuardsCheckEnd, '/team/22'],
+               [GuardsCheckStart, '/team/22'], [ChildActivationStart], [GuardsCheckEnd, '/team/22'],
                [NavigationCancel, '/team/22']
              ]);
-             expect((recordedEvents[3] as GuardsCheckEnd).shouldActivate).toBe(false);
+             expect((recordedEvents[4] as GuardsCheckEnd).shouldActivate).toBe(false);
            })));
       });
 
@@ -2389,10 +2392,12 @@ describe('Integration', () => {
                  [RoutesRecognized, '/lazyTrue/loaded'],
                  [GuardsCheckStart, '/lazyTrue/loaded'],
                  [ChildActivationStart],
+                 [ChildActivationStart],
                  [GuardsCheckEnd, '/lazyTrue/loaded'],
                  [ResolveStart, '/lazyTrue/loaded'],
                  [ResolveEnd, '/lazyTrue/loaded'],
                  [ChildActivationEnd],
+                 [ChildActivationEnd],
                  [NavigationEnd, '/lazyTrue/loaded'],
                ]);
              })));
@@ -2423,8 +2428,9 @@ describe('Integration', () => {
              [NavigationCancel, '/lazyFalse/loaded'],
 
              [NavigationStart, '/blank'], [RoutesRecognized, '/blank'],
-             [GuardsCheckStart, '/blank'], [GuardsCheckEnd, '/blank'], [ResolveStart, '/blank'],
+             [GuardsCheckStart, '/blank'], [ChildActivationStart], [GuardsCheckEnd, '/blank'],
-             [ResolveEnd, '/blank'], [NavigationEnd, '/blank']
+             [ResolveStart, '/blank'], [ResolveEnd, '/blank'], [ChildActivationEnd],
+             [NavigationEnd, '/blank']
            ]);
          })));
 

packages/router/test/router.spec.ts

@@ -10,6 +10,7 @@ import {Location} from '@angular/common';
 import {TestBed, inject} from '@angular/core/testing';
 
 import {ResolveData} from '../src/config';
+import {ChildActivationStart} from '../src/events';
 import {PreActivation} from '../src/pre_activation';
 import {Router} from '../src/router';
 import {ChildrenOutletContexts} from '../src/router_outlet_context';
@@ -60,6 +61,7 @@ describe('Router', () => {
     const inj = {get: (token: any) => () => `${token}_value`};
     let empty: RouterStateSnapshot;
     let logger: Logger;
+    let events: any[];
 
     const CA_CHILD = 'canActivate_child';
     const CA_CHILD_FALSE = 'canActivate_child_false';
@@ -88,8 +90,144 @@ describe('Router', () => {
     beforeEach(inject([Logger], (_logger: Logger) => {
       empty = createEmptyStateSnapshot(serializer.parse('/'), null !);
       logger = _logger;
+      events = [];
     }));
 
+    describe('ChildActivation', () => {
+      it('should run', () => {
+        /**
+         * R  -->  R (ChildActivationStart)
+         *          \
+         *           child
+         */
+        let result = false;
+        const childSnapshot =
+            createActivatedRouteSnapshot({component: 'child', routeConfig: {path: 'child'}});
+        const futureState = new RouterStateSnapshot(
+            'url', new TreeNode(empty.root, [new TreeNode(childSnapshot, [])]));
+
+        const p = new PreActivation(futureState, empty, TestBed, (evt) => { events.push(evt); });
+        p.initalize(new ChildrenOutletContexts());
+        p.checkGuards().subscribe((x) => result = x, (e) => { throw e; });
+        expect(result).toBe(true);
+        expect(events.length).toEqual(1);
+      });
+
+      it('should run from top to bottom', () => {
+        /**
+         * R  -->  R (ChildActivationStart)
+         *          \
+         *           child (ChildActivationStart)
+         *            \
+         *             grandchild (ChildActivationStart)
+         *              \
+         *               great grandchild
+         */
+        let result = false;
+        const childSnapshot =
+            createActivatedRouteSnapshot({component: 'child', routeConfig: {path: 'child'}});
+        const grandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'grandchild', routeConfig: {path: 'grandchild'}});
+        const greatGrandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'great-grandchild', routeConfig: {path: 'great-grandchild'}});
+        const futureState = new RouterStateSnapshot(
+            'url',
+            new TreeNode(
+                empty.root, [new TreeNode(childSnapshot, [
+                  new TreeNode(grandchildSnapshot, [new TreeNode(greatGrandchildSnapshot, [])])
+                ])]));
+
+        const p = new PreActivation(futureState, empty, TestBed, (evt) => { events.push(evt); });
+        p.initalize(new ChildrenOutletContexts());
+        p.checkGuards().subscribe((x) => result = x, (e) => { throw e; });
+
+        expect(result).toBe(true);
+        expect(events.length).toEqual(3);
+        expect(events[0].snapshot).toBe(events[0].snapshot.root);
+        expect(events[1].snapshot.routeConfig.path).toBe('child');
+        expect(events[2].snapshot.routeConfig.path).toBe('grandchild');
+      });
+
+      it('should not run for unchanged routes', () => {
+        /**
+         *         R  -->  R
+         *        / \
+         *   child   child (ChildActivationStart)
+         *            \
+         *             grandchild
+         */
+        let result = false;
+        const childSnapshot =
+            createActivatedRouteSnapshot({component: 'child', routeConfig: {path: 'child'}});
+        const grandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'grandchild', routeConfig: {path: 'grandchild'}});
+        const currentState = new RouterStateSnapshot(
+            'url', new TreeNode(empty.root, [new TreeNode(childSnapshot, [])]));
+        const futureState = new RouterStateSnapshot(
+            'url',
+            new TreeNode(
+                empty.root, [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])]));
+
+        const p =
+            new PreActivation(futureState, currentState, TestBed, (evt) => { events.push(evt); });
+        p.initalize(new ChildrenOutletContexts());
+        p.checkGuards().subscribe((x) => result = x, (e) => { throw e; });
+
+        expect(result).toBe(true);
+        expect(events.length).toEqual(1);
+        expect(events[0].snapshot).not.toBe(events[0].snapshot.root);
+        expect(events[0].snapshot.routeConfig.path).toBe('child');
+      });
+
+      it('should skip multiple unchanged routes but fire for all changed routes', () => {
+        /**
+         *         R  -->  R
+         *            / \
+         *       child   child
+         *          /     \
+         * grandchild      grandchild (ChildActivationStart)
+         *                  \
+         *                   greatgrandchild (ChildActivationStart)
+         *                    \
+         *                     great-greatgrandchild
+         */
+        let result = false;
+        const childSnapshot =
+            createActivatedRouteSnapshot({component: 'child', routeConfig: {path: 'child'}});
+        const grandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'grandchild', routeConfig: {path: 'grandchild'}});
+        const greatGrandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'greatgrandchild', routeConfig: {path: 'greatgrandchild'}});
+        const greatGreatGrandchildSnapshot = createActivatedRouteSnapshot(
+            {component: 'great-greatgrandchild', routeConfig: {path: 'great-greatgrandchild'}});
+        const currentState = new RouterStateSnapshot(
+            'url',
+            new TreeNode(
+                empty.root, [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])]));
+        const futureState = new RouterStateSnapshot(
+            'url',
+            new TreeNode(
+                empty.root,
+                [new TreeNode(
+                    childSnapshot, [new TreeNode(grandchildSnapshot, [
+                      new TreeNode(
+                          greatGrandchildSnapshot, [new TreeNode(greatGreatGrandchildSnapshot, [])])
+                    ])])]));
+
+        const p =
+            new PreActivation(futureState, currentState, TestBed, (evt) => { events.push(evt); });
+        p.initalize(new ChildrenOutletContexts());
+        p.checkGuards().subscribe((x) => result = x, (e) => { throw e; });
+
+        expect(result).toBe(true);
+        expect(events.length).toEqual(2);
+        expect(events[0] instanceof ChildActivationStart).toBe(true);
+        expect(events[0].snapshot).not.toBe(events[0].snapshot.root);
+        expect(events[0].snapshot.routeConfig.path).toBe('grandchild');
+        expect(events[1].snapshot.routeConfig.path).toBe('greatgrandchild');
+      });
+    });
+
     describe('guards', () => {
       it('should run CanActivate checks', () => {
         /**

tools/public_api_guard/router/index.d.ts

@@ -61,17 +61,17 @@ export interface CanLoad {
 
 /** @experimental */
 export declare class ChildActivationEnd {
-    route: Route;
+    snapshot: ActivatedRouteSnapshot;
     constructor(
-        route: Route);
+        snapshot: ActivatedRouteSnapshot);
     toString(): string;
 }
 
 /** @experimental */
 export declare class ChildActivationStart {
-    route: Route;
+    snapshot: ActivatedRouteSnapshot;
     constructor(
-        route: Route);
+        snapshot: ActivatedRouteSnapshot);
     toString(): string;
 }