Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router): inconsistent router state, missing execution of data resolvers #52934

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1883,9 +1883,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));
leonelvsc marked this conversation as resolved.
Show resolved Hide resolved
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