fix(router): pass correct component to canDeactivate checks when using two or more sibling router-outlets (#36302)

fixes #34614

There's an edge case where if I use two (or more) sibling <router-outlet>s in two (or more) child routes where their parent route doesn't have a component then preactivation will trigger all canDeactivate checks with the same component because it will use wrong OutletContext.

PR Close #36302
This commit is contained in:
Martin Sikora 2020-03-29 11:13:27 +02:00 committed by atscott
parent 34aa5570ed
commit 80e6c07d89
2 changed files with 57 additions and 15 deletions

View File

@ -10,10 +10,10 @@ import {Injector} from '@angular/core';
import {LoadedRouterConfig, RunGuardsAndResolvers} from '../config'; import {LoadedRouterConfig, RunGuardsAndResolvers} from '../config';
import {ChildrenOutletContexts, OutletContext} from '../router_outlet_context'; import {ChildrenOutletContexts, OutletContext} from '../router_outlet_context';
import {ActivatedRouteSnapshot, RouterStateSnapshot, equalParamsAndUrlSegments} from '../router_state'; import {ActivatedRouteSnapshot, equalParamsAndUrlSegments, RouterStateSnapshot} from '../router_state';
import {equalPath} from '../url_tree'; import {equalPath} from '../url_tree';
import {forEach, shallowEqual} from '../utils/collection'; import {forEach, shallowEqual} from '../utils/collection';
import {TreeNode, nodeChildrenAsMap} from '../utils/tree'; import {nodeChildrenAsMap, TreeNode} from '../utils/tree';
export class CanActivate { export class CanActivate {
readonly route: ActivatedRouteSnapshot; readonly route: ActivatedRouteSnapshot;
@ -66,9 +66,8 @@ function getClosestLoadedConfig(snapshot: ActivatedRouteSnapshot): LoadedRouterC
} }
function getChildRouteGuards( function getChildRouteGuards(
futureNode: TreeNode<ActivatedRouteSnapshot>, currNode: TreeNode<ActivatedRouteSnapshot>| null, futureNode: TreeNode<ActivatedRouteSnapshot>, currNode: TreeNode<ActivatedRouteSnapshot>|null,
contexts: ChildrenOutletContexts | null, futurePath: ActivatedRouteSnapshot[], contexts: ChildrenOutletContexts|null, futurePath: ActivatedRouteSnapshot[], checks: Checks = {
checks: Checks = {
canDeactivateChecks: [], canDeactivateChecks: [],
canActivateChecks: [] canActivateChecks: []
}): Checks { }): Checks {
@ -82,15 +81,16 @@ function getChildRouteGuards(
// Process any children left from the current route (not active for the future route) // Process any children left from the current route (not active for the future route)
forEach( forEach(
prevChildren, (v: TreeNode<ActivatedRouteSnapshot>, k: string) => prevChildren,
deactivateRouteAndItsChildren(v, contexts !.getContext(k), checks)); (v: TreeNode<ActivatedRouteSnapshot>, k: string) =>
deactivateRouteAndItsChildren(v, contexts!.getContext(k), contexts, checks));
return checks; return checks;
} }
function getRouteGuards( function getRouteGuards(
futureNode: TreeNode<ActivatedRouteSnapshot>, currNode: TreeNode<ActivatedRouteSnapshot>, futureNode: TreeNode<ActivatedRouteSnapshot>, currNode: TreeNode<ActivatedRouteSnapshot>,
parentContexts: ChildrenOutletContexts | null, futurePath: ActivatedRouteSnapshot[], parentContexts: ChildrenOutletContexts|null, futurePath: ActivatedRouteSnapshot[],
checks: Checks = { checks: Checks = {
canDeactivateChecks: [], canDeactivateChecks: [],
canActivateChecks: [] canActivateChecks: []
@ -102,7 +102,7 @@ function getRouteGuards(
// reusing the node // reusing the node
if (curr && future.routeConfig === curr.routeConfig) { if (curr && future.routeConfig === curr.routeConfig) {
const shouldRun = const shouldRun =
shouldRunGuardsAndResolvers(curr, future, future.routeConfig !.runGuardsAndResolvers); shouldRunGuardsAndResolvers(curr, future, future.routeConfig!.runGuardsAndResolvers);
if (shouldRun) { if (shouldRun) {
checks.canActivateChecks.push(new CanActivate(futurePath)); checks.canActivateChecks.push(new CanActivate(futurePath));
} else { } else {
@ -127,7 +127,7 @@ function getRouteGuards(
} }
} else { } else {
if (curr) { if (curr) {
deactivateRouteAndItsChildren(currNode, context, checks); deactivateRouteAndItsChildren(currNode, context, parentContexts, checks);
} }
checks.canActivateChecks.push(new CanActivate(futurePath)); checks.canActivateChecks.push(new CanActivate(futurePath));
@ -146,7 +146,7 @@ function getRouteGuards(
function shouldRunGuardsAndResolvers( function shouldRunGuardsAndResolvers(
curr: ActivatedRouteSnapshot, future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot, future: ActivatedRouteSnapshot,
mode: RunGuardsAndResolvers | undefined): boolean { mode: RunGuardsAndResolvers|undefined): boolean {
if (typeof mode === 'function') { if (typeof mode === 'function') {
return mode(curr, future); return mode(curr, future);
} }
@ -172,17 +172,21 @@ function shouldRunGuardsAndResolvers(
} }
function deactivateRouteAndItsChildren( function deactivateRouteAndItsChildren(
route: TreeNode<ActivatedRouteSnapshot>, context: OutletContext | null, checks: Checks): void { route: TreeNode<ActivatedRouteSnapshot>, context: OutletContext|null,
parentContexts: ChildrenOutletContexts|null, checks: Checks): void {
const children = nodeChildrenAsMap(route); const children = nodeChildrenAsMap(route);
const r = route.value; const r = route.value;
forEach(children, (node: TreeNode<ActivatedRouteSnapshot>, childName: string) => { forEach(children, (node: TreeNode<ActivatedRouteSnapshot>, childName: string) => {
if (!r.component) { if (!r.component) {
deactivateRouteAndItsChildren(node, context, checks); deactivateRouteAndItsChildren(
node, parentContexts ? parentContexts.getContext(childName) : context, parentContexts,
checks);
} else if (context) { } else if (context) {
deactivateRouteAndItsChildren(node, context.children.getContext(childName), checks); deactivateRouteAndItsChildren(
node, context.children.getContext(childName), parentContexts, checks);
} else { } else {
deactivateRouteAndItsChildren(node, null, checks); deactivateRouteAndItsChildren(node, null, parentContexts, checks);
} }
}); });

View File

@ -3046,6 +3046,44 @@ describe('Integration', () => {
expect(location.path()).toEqual('/two-outlets/(a)'); expect(location.path()).toEqual('/two-outlets/(a)');
}))); })));
it('should call canDeactivate handler with each deactivated component',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, TwoOutletsCmp);
router.resetConfig([
{
path: 'a',
children: [
{
path: 'b1',
component: BlankCmp,
canDeactivate: ['RecordingDeactivate'],
},
{
path: 'b2',
canDeactivate: ['RecordingDeactivate'],
component: SimpleCmp,
outlet: 'aux',
},
],
},
{
path: 'c',
component: BlankCmp,
},
]);
router.navigate(['/a', {outlets: {primary: ['b1'], aux: ['b2']}}]);
advance(fixture);
expect(location.path()).toEqual('/a/(b1//aux:b2)');
router.navigate(['/c']);
advance(fixture);
expect(log[0].component).toBeAnInstanceOf(BlankCmp);
expect(log[1].component).toBeAnInstanceOf(SimpleCmp);
})));
it('works with a nested route', it('works with a nested route',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => { fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);