fix(router): Resolvers in different parts of the route tree should be able to execute together (#52934)

leonelvsc · AndrewKushnir · commit 91486aaf0754 · 2023-11-20T12:07:11.000-08:00
The following commit accidentally broken execution of resolvers when two resolvers appear in different parts of the tree and do not share a 3278966 This happens when there are secondary routes. This test ensures that all routes with resolves are run. fixes #52892 PR Close #52934
diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -1877,9 +1877,6 @@
   {
     "name": "resetPreOrderHookFlags"
   },
-  {
-    "name": "resolveData"
-  },
   {
     "name": "resolveForwardRef"
   },
diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts
@@ -28,21 +28,25 @@ export function resolveData(
     if (!canActivateChecks.length) {
       return of(t);
     }
-    const routesWithResolversToRun = canActivateChecks.map(check => check.route);
-    const routesWithResolversSet = new Set(routesWithResolversToRun);
-    const routesNeedingDataUpdates =
-        // List all ActivatedRoutes in an array, starting from the parent of the first route to run
-        // resolvers. We go from the parent because the first route might have siblings that also
-        // run resolvers.
-        flattenRouteTree(routesWithResolversToRun[0].parent!)
-            // Remove the parent from the list -- we do not need to recompute its inherited data
-            // because no resolvers at or above it run.
-            .slice(1);
+    // Iterating a Set in javascript  happens in insertion order so it is safe to use a `Set` to
+    // preserve the correct order that the resolvers should run in.
+    // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#description
+    const routesWithResolversToRun = new Set(canActivateChecks.map(check => check.route));
+    const routesNeedingDataUpdates = new Set<ActivatedRouteSnapshot>();
+    for (const route of routesWithResolversToRun) {
+      if (routesNeedingDataUpdates.has(route)) {
+        continue;
+      }
+      // All children under the route with a resolver to run need to recompute inherited data.
+      for (const newRoute of flattenRouteTree(route)) {
+        routesNeedingDataUpdates.add(newRoute);
+      }
+    }
     let routesProcessed = 0;
     return from(routesNeedingDataUpdates)
         .pipe(
             concatMap(route => {
-              if (routesWithResolversSet.has(route)) {
+              if (routesWithResolversToRun.has(route)) {
                 return runResolve(route, targetSnapshot!, paramsInheritanceStrategy, injector);
               } else {
                 route.data = getInherited(route, route.parent, paramsInheritanceStrategy).resolve;
@@ -51,7 +55,7 @@ export function resolveData(
             }),
             tap(() => routesProcessed++),
             takeLast(1),
-            mergeMap(_ => routesProcessed === routesNeedingDataUpdates.length ? of(t) : EMPTY),
+            mergeMap(_ => routesProcessed === routesNeedingDataUpdates.size ? of(t) : EMPTY),
         );
   });
 }
diff --git a/packages/router/test/operators/resolve_data.spec.ts b/packages/router/test/operators/resolve_data.spec.ts
@@ -46,6 +46,44 @@ describe('resolveData operator', () => {
     expect(TestBed.inject(Router).url).toEqual('/');
   });
 
+  it('should run resolvers in different parts of the tree', async () => {
+    let value = 0;
+    let bValue = 0;
+    TestBed.configureTestingModule({
+      providers: [provideRouter([
+        {
+          path: 'a',
+          runGuardsAndResolvers: () => false,
+          children: [{
+            path: '',
+            resolve: {d0: () => ++value},
+            runGuardsAndResolvers: 'always',
+            children: [],
+          }],
+        },
+        {
+          path: 'b',
+          outlet: 'aux',
+          runGuardsAndResolvers: () => false,
+          children: [{
+            path: '',
+            resolve: {d1: () => ++bValue},
+            runGuardsAndResolvers: 'always',
+            children: [],
+          }]
+        },
+      ])]
+    });
+    const router = TestBed.inject(Router);
+    const harness = await RouterTestingHarness.create('/a(aux:b)');
+    expect(router.routerState.root.children[0]?.firstChild?.snapshot.data).toEqual({d0: 1});
+    expect(router.routerState.root.children[1]?.firstChild?.snapshot.data).toEqual({d1: 1});
+
+    await harness.navigateByUrl('/a(aux:b)#new');
+    expect(router.routerState.root.children[0]?.firstChild?.snapshot.data).toEqual({d0: 2});
+    expect(router.routerState.root.children[1]?.firstChild?.snapshot.data).toEqual({d1: 2});
+  });
+
   it('should update children inherited data when resolvers run', async () => {
     let value = 0;
     TestBed.configureTestingModule({

Original file line number	Diff line number	Diff line change
`@@ -1877,9 +1877,6 @@`
`1877`	`1877`	`{`
`1878`	`1878`	`"name": "resetPreOrderHookFlags"`
`1879`	`1879`	`},`
`1880`		`- {`
`1881`		`- "name": "resolveData"`
`1882`		`- },`
`1883`	`1880`	`{`
`1884`	`1881`	`"name": "resolveForwardRef"`
`1885`	`1882`	`},`