Skip to content

Commit

Permalink
fix(router): Resolvers in different parts of the route tree should be…
Browse files Browse the repository at this point in the history
… able to execute together (#52934)

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
  • Loading branch information
leonelvsc authored and AndrewKushnir committed Nov 20, 2023
1 parent b35c673 commit 29e0834
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 15 deletions.
3 changes: 0 additions & 3 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1886,9 +1886,6 @@
{
"name": "resetPreOrderHookFlags"
},
{
"name": "resolveData"
},
{
"name": "resolveForwardRef"
},
Expand Down
28 changes: 16 additions & 12 deletions packages/router/src/operators/resolve_data.ts
Expand Up @@ -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;
Expand All @@ -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),
);
});
}
Expand Down
38 changes: 38 additions & 0 deletions packages/router/test/operators/resolve_data.spec.ts
Expand Up @@ -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({
Expand Down

0 comments on commit 29e0834

Please sign in to comment.