From 9ddf9b3d3dfb0e7d7c23761b1e8ed75effb85dc1 Mon Sep 17 00:00:00 2001
From: Victor Savkin <vic.savkin@gmail.com>
Date: Thu, 3 Nov 2016 16:26:10 -0700
Subject: [PATCH] fix(router): advance a route only after its children have
 been deactivated (#12676)

Closes #11715
---
 modules/@angular/router/src/router.ts         | 48 +++++++++++++---
 .../@angular/router/test/integration.spec.ts  | 56 +++++++++++++++++++
 2 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts
index 569ed34a12..903f1b1d08 100644
--- a/modules/@angular/router/src/router.ts
+++ b/modules/@angular/router/src/router.ts
@@ -984,19 +984,53 @@ class ActivateRoutes {
   activate(parentOutletMap: RouterOutletMap): void {
     const futureRoot = this.futureState._root;
     const currRoot = this.currState ? this.currState._root : null;
+
+    this.deactivateChildRoutes(futureRoot, currRoot, parentOutletMap);
     advanceActivatedRoute(this.futureState.root);
     this.activateChildRoutes(futureRoot, currRoot, parentOutletMap);
   }
 
+  private deactivateChildRoutes(
+      futureNode: TreeNode<ActivatedRoute>, currNode: TreeNode<ActivatedRoute>,
+      outletMap: RouterOutletMap): void {
+    const prevChildren: {[key: string]: any} = nodeChildrenAsMap(currNode);
+    futureNode.children.forEach(c => {
+      this.deactivateRoutes(c, prevChildren[c.value.outlet], outletMap);
+      delete prevChildren[c.value.outlet];
+    });
+    forEach(prevChildren, (v: any, k: string) => this.deactiveRouteAndItsChildren(v, outletMap));
+  }
+
   private activateChildRoutes(
       futureNode: TreeNode<ActivatedRoute>, currNode: TreeNode<ActivatedRoute>,
       outletMap: RouterOutletMap): void {
     const prevChildren: {[key: string]: any} = nodeChildrenAsMap(currNode);
-    futureNode.children.forEach(c => {
-      this.activateRoutes(c, prevChildren[c.value.outlet], outletMap);
-      delete prevChildren[c.value.outlet];
-    });
-    forEach(prevChildren, (v: any, k: string) => this.deactiveRouteAndItsChildren(v, outletMap));
+    futureNode.children.forEach(
+        c => { this.activateRoutes(c, prevChildren[c.value.outlet], outletMap); });
+  }
+
+  deactivateRoutes(
+      futureNode: TreeNode<ActivatedRoute>, currNode: TreeNode<ActivatedRoute>,
+      parentOutletMap: RouterOutletMap): void {
+    const future = futureNode.value;
+    const curr = currNode ? currNode.value : null;
+
+    // reusing the node
+    if (future === curr) {
+      // If we have a normal route, we need to go through an outlet.
+      if (future.component) {
+        const outlet = getOutlet(parentOutletMap, future);
+        this.deactivateChildRoutes(futureNode, currNode, outlet.outletMap);
+
+        // if we have a componentless route, we recurse but keep the same outlet map.
+      } else {
+        this.deactivateChildRoutes(futureNode, currNode, parentOutletMap);
+      }
+    } else {
+      if (curr) {
+        this.deactiveRouteAndItsChildren(currNode, parentOutletMap);
+      }
+    }
   }
 
   activateRoutes(
@@ -1020,10 +1054,6 @@ class ActivateRoutes {
         this.activateChildRoutes(futureNode, currNode, parentOutletMap);
       }
     } else {
-      if (curr) {
-        this.deactiveRouteAndItsChildren(currNode, parentOutletMap);
-      }
-
       // if we have a normal route, we need to advance the route
       // and place the component into the outlet. After that recurse.
       if (future.component) {
diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts
index b107470189..0b02b88160 100644
--- a/modules/@angular/router/test/integration.spec.ts
+++ b/modules/@angular/router/test/integration.spec.ts
@@ -72,6 +72,62 @@ describe('Integration', () => {
       });
     });
 
+    describe('should advance the parent route after deactivating its children', () => {
+      let log: string[] = [];
+
+      @Component({template: '<router-outlet></router-outlet>'})
+      class Parent {
+        constructor(route: ActivatedRoute) {
+          route.params.subscribe((s: any) => { log.push(s); });
+        }
+      }
+
+      @Component({template: 'child1'})
+      class Child1 {
+        ngOnDestroy() { log.push('child1 destroy'); }
+      }
+
+      @Component({template: 'child2'})
+      class Child2 {
+        constructor() { log.push('child2 constructor'); }
+      }
+
+      @NgModule({
+        declarations: [Parent, Child1, Child2],
+        entryComponents: [Parent, Child1, Child2],
+        imports: [RouterModule]
+      })
+      class TestModule {
+      }
+
+      beforeEach(() => {
+        log = [];
+        TestBed.configureTestingModule({imports: [TestModule]});
+      });
+
+      it('should work',
+         fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
+           const fixture = createRoot(router, RootCmp);
+
+           router.resetConfig([{
+             path: 'parent/:id',
+             component: Parent,
+             children:
+                 [{path: 'child1', component: Child1}, {path: 'child2', component: Child2}]
+           }]);
+
+           router.navigateByUrl('/parent/1/child1');
+           advance(fixture);
+
+           router.navigateByUrl('/parent/2/child2');
+           advance(fixture);
+
+           expect(location.path()).toEqual('/parent/2/child2');
+           expect(log).toEqual([{id: '1'}, 'child1 destroy', {id: '2'}, 'child2 constructor']);
+         })));
+
+    });
+
     it('should execute navigations serialy',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = createRoot(router, RootCmp);