fix(router): correctly deactivate children with componentless parent (#…

…40196)

During route activation, a componentless route will not have a context created
for it, but the logic continues to recurse so that children are still
activated. This can be seen here:
https://github.com/angular/angular/blob/362f45c4bf1bb49a90b014d2053f4c4474d132c0/packages/router/src/operators/activate_routes.ts#L151-L158

The current deactivation logic does not currently account for componentless routes.

This commit adjusts the deactivation logic so that if a context cannot
be retrieved for a given route (because it is componentless), we
continue to recurse and deactivate the children using the same
`parentContexts` in the same way that activation does.

Fixes #20694

PR Close #40196

packages/router/src/operators/activate_routes.ts

@@ -109,26 +109,27 @@ export class ActivateRoutes {
   private deactivateRouteAndOutlet(
       route: TreeNode<ActivatedRoute>, parentContexts: ChildrenOutletContexts): void {
     const context = parentContexts.getContext(route.value.outlet);
+    // The context could be `null` if we are on a componentless route but there may still be
+    // children that need deactivating.
+    const contexts = context && route.value.component ? context.children : parentContexts;
+    const children: {[outletName: string]: TreeNode<ActivatedRoute>} = nodeChildrenAsMap(route);
 
-    if (context) {
-      const children: {[outletName: string]: any} = nodeChildrenAsMap(route);
-      const contexts = route.value.component ? context.children : parentContexts;
-
-      forEach(children, (v: any, k: string) => this.deactivateRouteAndItsChildren(v, contexts));
+    for (const child of Object.values(children)) {
+      this.deactivateRouteAndItsChildren(child, contexts);
+    }
 
-      if (context.outlet) {
-        // Destroy the component
-        context.outlet.deactivate();
-        // Destroy the contexts for all the outlets that were in the component
-        context.children.onOutletDeactivated();
-      }
+    if (context && context.outlet) {
+      // Destroy the component
+      context.outlet.deactivate();
+      // Destroy the contexts for all the outlets that were in the component
+      context.children.onOutletDeactivated();
     }
   }
 
   private activateChildRoutes(
       futureNode: TreeNode<ActivatedRoute>, currNode: TreeNode<ActivatedRoute>|null,
       contexts: ChildrenOutletContexts): void {
-    const children: {[outlet: string]: any} = nodeChildrenAsMap(currNode);
+    const children: {[outlet: string]: TreeNode<ActivatedRoute>} = nodeChildrenAsMap(currNode);
     futureNode.children.forEach(c => {
       this.activateRoutes(c, children[c.value.outlet], contexts);
       this.forwardEvent(new ActivationEnd(c.value.snapshot));

packages/router/test/integration.spec.ts

@@ -389,7 +389,7 @@ describe('Integration', () => {
       });
     });
 
-    describe('should advance the parent route after deactivating its children', () => {
+    describe('route activation', () => {
       @Component({template: '<router-outlet></router-outlet>'})
       class Parent {
         constructor(route: ActivatedRoute) {
@@ -399,8 +399,24 @@ describe('Integration', () => {
         }
       }
 
+      @Component({
+        template: `
+        <router-outlet (deactivate)="logDeactivate('primary')"></router-outlet>
+        <router-outlet name="first" (deactivate)="logDeactivate('first')"></router-outlet>
+        <router-outlet name="second" (deactivate)="logDeactivate('second')"></router-outlet>
+        `
+      })
+      class NamedOutletHost {
+        logDeactivate(route: string) {
+          log.push(route + ' deactivate');
+        }
+      }
+
       @Component({template: 'child1'})
       class Child1 {
+        constructor() {
+          log.push('child1 constructor');
+        }
         ngOnDestroy() {
           log.push('child1 destroy');
         }
@@ -411,20 +427,33 @@ describe('Integration', () => {
         constructor() {
           log.push('child2 constructor');
         }
+        ngOnDestroy() {
+          log.push('child2 destroy');
+        }
+      }
+
+      @Component({template: 'child3'})
+      class Child3 {
+        constructor() {
+          log.push('child3 constructor');
+        }
+        ngOnDestroy() {
+          log.push('child3 destroy');
+        }
       }
 
       @NgModule({
-        declarations: [Parent, Child1, Child2],
-        entryComponents: [Parent, Child1, Child2],
+        declarations: [Parent, NamedOutletHost, Child1, Child2, Child3],
+        entryComponents: [Parent, NamedOutletHost, Child1, Child2, Child3],
         imports: [RouterModule]
       })
       class TestModule {
       }
 
-      beforeEach(() => TestBed.configureTestingModule({imports: [TestModule]}));
-
-      it('should work',
-         fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
+      it('should advance the parent route after deactivating its children', fakeAsync(() => {
+           TestBed.configureTestingModule({imports: [TestModule]});
+           const router = TestBed.inject(Router);
+           const location = TestBed.inject(Location);
            const fixture = createRoot(router, RootCmp);
 
            router.resetConfig([{
@@ -445,11 +474,81 @@ describe('Integration', () => {
            expect(location.path()).toEqual('/parent/2/child2');
            expect(log).toEqual([
              {id: '1'},
+             'child1 constructor',
              'child1 destroy',
              {id: '2'},
              'child2 constructor',
            ]);
-         })));
+         }));
+
+      it('should deactivate outlet children with componentless parent', fakeAsync(() => {
+           TestBed.configureTestingModule({imports: [TestModule]});
+           const router = TestBed.inject(Router);
+           const fixture = createRoot(router, RootCmp);
+
+           router.resetConfig([
+             {
+               path: 'named-outlets',
+               component: NamedOutletHost,
+               children: [
+                 {
+                   path: 'home',
+                   children: [
+                     {path: '', component: Child1, outlet: 'first'},
+                     {path: '', component: Child2, outlet: 'second'},
+                     {path: 'primary', component: Child3},
+                   ]
+                 },
+                 {
+                   path: 'about',
+                   children: [
+                     {path: '', component: Child1, outlet: 'first'},
+                     {path: '', component: Child2, outlet: 'second'},
+                   ]
+                 },
+
+               ]
+             },
+             {
+               path: 'other',
+               component: Parent,
+             },
+           ]);
+
+           router.navigateByUrl('/named-outlets/home/primary');
+           advance(fixture);
+           expect(log).toEqual([
+             'child3 constructor',  // primary outlet always first
+             'child1 constructor',
+             'child2 constructor',
+           ]);
+           log.length = 0;
+
+           router.navigateByUrl('/named-outlets/about');
+           advance(fixture);
+           expect(log).toEqual([
+             'child3 destroy',
+             'primary deactivate',
+             'child1 destroy',
+             'first deactivate',
+             'child2 destroy',
+             'second deactivate',
+             'child1 constructor',
+             'child2 constructor',
+           ]);
+           log.length = 0;
+
+           router.navigateByUrl('/other');
+           advance(fixture);
+           expect(log).toEqual([
+             'child1 destroy',
+             'first deactivate',
+             'child2 destroy',
+             'second deactivate',
+             // route param subscription from 'Parent' component
+             {},
+           ]);
+         }));
     });
 
     it('should not wait for prior navigations to start a new navigation',