From 80e6c07d89d74d71956fb1299c54f5353e255370 Mon Sep 17 00:00:00 2001 From: Martin Sikora Date: Sun, 29 Mar 2020 11:13:27 +0200 Subject: [PATCH] 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 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 --- packages/router/src/utils/preactivation.ts | 34 ++++++++++--------- packages/router/test/integration.spec.ts | 38 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/packages/router/src/utils/preactivation.ts b/packages/router/src/utils/preactivation.ts index 1899a0575b..a3624a152f 100644 --- a/packages/router/src/utils/preactivation.ts +++ b/packages/router/src/utils/preactivation.ts @@ -10,10 +10,10 @@ import {Injector} from '@angular/core'; import {LoadedRouterConfig, RunGuardsAndResolvers} from '../config'; 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 {forEach, shallowEqual} from '../utils/collection'; -import {TreeNode, nodeChildrenAsMap} from '../utils/tree'; +import {nodeChildrenAsMap, TreeNode} from '../utils/tree'; export class CanActivate { readonly route: ActivatedRouteSnapshot; @@ -66,9 +66,8 @@ function getClosestLoadedConfig(snapshot: ActivatedRouteSnapshot): LoadedRouterC } function getChildRouteGuards( - futureNode: TreeNode, currNode: TreeNode| null, - contexts: ChildrenOutletContexts | null, futurePath: ActivatedRouteSnapshot[], - checks: Checks = { + futureNode: TreeNode, currNode: TreeNode|null, + contexts: ChildrenOutletContexts|null, futurePath: ActivatedRouteSnapshot[], checks: Checks = { canDeactivateChecks: [], canActivateChecks: [] }): Checks { @@ -82,15 +81,16 @@ function getChildRouteGuards( // Process any children left from the current route (not active for the future route) forEach( - prevChildren, (v: TreeNode, k: string) => - deactivateRouteAndItsChildren(v, contexts !.getContext(k), checks)); + prevChildren, + (v: TreeNode, k: string) => + deactivateRouteAndItsChildren(v, contexts!.getContext(k), contexts, checks)); return checks; } function getRouteGuards( futureNode: TreeNode, currNode: TreeNode, - parentContexts: ChildrenOutletContexts | null, futurePath: ActivatedRouteSnapshot[], + parentContexts: ChildrenOutletContexts|null, futurePath: ActivatedRouteSnapshot[], checks: Checks = { canDeactivateChecks: [], canActivateChecks: [] @@ -102,7 +102,7 @@ function getRouteGuards( // reusing the node if (curr && future.routeConfig === curr.routeConfig) { const shouldRun = - shouldRunGuardsAndResolvers(curr, future, future.routeConfig !.runGuardsAndResolvers); + shouldRunGuardsAndResolvers(curr, future, future.routeConfig!.runGuardsAndResolvers); if (shouldRun) { checks.canActivateChecks.push(new CanActivate(futurePath)); } else { @@ -127,7 +127,7 @@ function getRouteGuards( } } else { if (curr) { - deactivateRouteAndItsChildren(currNode, context, checks); + deactivateRouteAndItsChildren(currNode, context, parentContexts, checks); } checks.canActivateChecks.push(new CanActivate(futurePath)); @@ -146,7 +146,7 @@ function getRouteGuards( function shouldRunGuardsAndResolvers( curr: ActivatedRouteSnapshot, future: ActivatedRouteSnapshot, - mode: RunGuardsAndResolvers | undefined): boolean { + mode: RunGuardsAndResolvers|undefined): boolean { if (typeof mode === 'function') { return mode(curr, future); } @@ -172,17 +172,21 @@ function shouldRunGuardsAndResolvers( } function deactivateRouteAndItsChildren( - route: TreeNode, context: OutletContext | null, checks: Checks): void { + route: TreeNode, context: OutletContext|null, + parentContexts: ChildrenOutletContexts|null, checks: Checks): void { const children = nodeChildrenAsMap(route); const r = route.value; forEach(children, (node: TreeNode, childName: string) => { if (!r.component) { - deactivateRouteAndItsChildren(node, context, checks); + deactivateRouteAndItsChildren( + node, parentContexts ? parentContexts.getContext(childName) : context, parentContexts, + checks); } else if (context) { - deactivateRouteAndItsChildren(node, context.children.getContext(childName), checks); + deactivateRouteAndItsChildren( + node, context.children.getContext(childName), parentContexts, checks); } else { - deactivateRouteAndItsChildren(node, null, checks); + deactivateRouteAndItsChildren(node, null, parentContexts, checks); } }); diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 358f4ccc46..1dcf40ecf9 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -3046,6 +3046,44 @@ describe('Integration', () => { 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', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { const fixture = createRoot(router, RootCmp);