From a1b2718b92457effbbdc2e31838f3cf96b0b8fe0 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Fri, 9 Apr 2021 14:58:55 -0700
Subject: [PATCH] fix(router): recursively merge empty path matches (#41584)

When recognizing routes, the router merges nodes which map to the same
empty path config. This is because auxiliary outlets under empty path
parents need to match the parent config. This would result in two
outlet matches for that parent which need to be combined into a single
node: The regular 'primary' match and the match for the auxiliary outlet.
In addition, the children of the merged nodes should also be merged to
account for multiple levels of empty path parents.

Fixes #41481

PR Close #41584
---
 packages/router/src/recognize.ts         | 13 +++++++++-
 packages/router/test/integration.spec.ts | 33 ++++++++++++++++++++++++
 packages/router/test/recognize.spec.ts   | 13 +++++++---
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts
index 64d3f25459..cb12aa8172 100644
--- a/packages/router/src/recognize.ts
+++ b/packages/router/src/recognize.ts
@@ -251,6 +251,8 @@ function hasEmptyPathConfig(node: TreeNode<ActivatedRouteSnapshot>) {
 function mergeEmptyPathMatches(nodes: Array<TreeNode<ActivatedRouteSnapshot>>):
     Array<TreeNode<ActivatedRouteSnapshot>> {
   const result: Array<TreeNode<ActivatedRouteSnapshot>> = [];
+  // The set of nodes which contain children that were merged from two duplicate empty path nodes.
+  const mergedNodes: Set<TreeNode<ActivatedRouteSnapshot>> = new Set();
 
   for (const node of nodes) {
     if (!hasEmptyPathConfig(node)) {
@@ -262,11 +264,20 @@ function mergeEmptyPathMatches(nodes: Array<TreeNode<ActivatedRouteSnapshot>>):
         result.find(resultNode => node.value.routeConfig === resultNode.value.routeConfig);
     if (duplicateEmptyPathNode !== undefined) {
       duplicateEmptyPathNode.children.push(...node.children);
+      mergedNodes.add(duplicateEmptyPathNode);
     } else {
       result.push(node);
     }
   }
-  return result;
+  // For each node which has children from multiple sources, we need to recompute a new `TreeNode`
+  // by also merging those children. This is necessary when there are multiple empty path configs in
+  // a row. Put another way: whenever we combine children of two nodes, we need to also check if any
+  // of those children can be combined into a single node as well.
+  for (const mergedNode of mergedNodes) {
+    const mergedChildren = mergeEmptyPathMatches(mergedNode.children);
+    result.push(new TreeNode(mergedNode.value, mergedChildren));
+  }
+  return result.filter(n => !mergedNodes.has(n));
 }
 
 function checkOutletNameUniqueness(nodes: TreeNode<ActivatedRouteSnapshot>[]): void {
diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
index ca5b677f54..01ccd5495e 100644
--- a/packages/router/test/integration.spec.ts
+++ b/packages/router/test/integration.spec.ts
@@ -595,6 +595,39 @@ describe('Integration', () => {
              {},
            ]);
          }));
+
+      it('should work between aux outlets under two levels of empty path parents', fakeAsync(() => {
+           TestBed.configureTestingModule({imports: [TestModule]});
+           const router = TestBed.inject(Router);
+           router.resetConfig([{
+             path: '',
+             children: [
+               {
+                 path: '',
+                 component: NamedOutletHost,
+                 children: [
+                   {path: 'one', component: Child1, outlet: 'first'},
+                   {path: 'two', component: Child2, outlet: 'first'},
+                 ]
+               },
+             ]
+           }]);
+
+           const fixture = createRoot(router, RootCmp);
+
+           router.navigateByUrl('/(first:one)');
+           advance(fixture);
+           expect(log).toEqual(['child1 constructor']);
+
+           log.length = 0;
+           router.navigateByUrl('/(first:two)');
+           advance(fixture);
+           expect(log).toEqual([
+             'child1 destroy',
+             'first deactivate',
+             'child2 constructor',
+           ]);
+         }));
     });
 
     it('should not wait for prior navigations to start a new navigation',
diff --git a/packages/router/test/recognize.spec.ts b/packages/router/test/recognize.spec.ts
index e692097332..eff0951df6 100644
--- a/packages/router/test/recognize.spec.ts
+++ b/packages/router/test/recognize.spec.ts
@@ -626,9 +626,16 @@ describe('recognize', () => {
               }]
             }],
             '(c:c)');
-        checkActivatedRoute(s.root.children[0], '', {}, ComponentA);
-        checkActivatedRoute(s.root.children[0].children[0], '', {}, ComponentB);
-        checkActivatedRoute(s.root.children[0].children[0].children[0], 'c', {}, ComponentC, 'c');
+        const [compAConfig] = s.root.children;
+        checkActivatedRoute(compAConfig, '', {}, ComponentA);
+        expect(compAConfig.children.length).toBe(1);
+
+        const [compBConfig] = compAConfig.children;
+        checkActivatedRoute(compBConfig, '', {}, ComponentB);
+        expect(compBConfig.children.length).toBe(1);
+
+        const [compCConfig] = compBConfig.children;
+        checkActivatedRoute(compCConfig, 'c', {}, ComponentC, 'c');
       });
 
       it('should not persist a primary segment beyond the boundary of a named outlet match', () => {