From 532e53678d423e29abff7855d5b6eef0ab5dd5eb Mon Sep 17 00:00:00 2001 From: Jason Aden Date: Wed, 3 Oct 2018 11:47:46 -0700 Subject: [PATCH] refactor(router): get guards only one time and simplify guard operator signature (#26239) PR Close #26239 --- packages/router/src/operators/check_guards.ts | 51 +++++++++---------- packages/router/src/operators/resolve_data.ts | 12 ++--- packages/router/src/router.ts | 19 ++++--- packages/router/test/router.spec.ts | 33 ++++++------ 4 files changed, 58 insertions(+), 57 deletions(-) diff --git a/packages/router/src/operators/check_guards.ts b/packages/router/src/operators/check_guards.ts index 370189966d..bf2d82f1b7 100644 --- a/packages/router/src/operators/check_guards.ts +++ b/packages/router/src/operators/check_guards.ts @@ -12,28 +12,27 @@ import {concatMap, every, first, map, mergeMap} from 'rxjs/operators'; import {ActivationStart, ChildActivationStart, Event} from '../events'; import {NavigationTransition} from '../router'; -import {ChildrenOutletContexts} from '../router_outlet_context'; import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state'; import {andObservables, wrapIntoObservable} from '../utils/collection'; -import {CanActivate, CanDeactivate, Checks, getAllRouteGuards, getCanActivateChild, getToken} from '../utils/preactivation'; +import {CanActivate, CanDeactivate, getCanActivateChild, getToken} from '../utils/preactivation'; -export function checkGuards( - rootContexts: ChildrenOutletContexts, moduleInjector: Injector, - forwardEvent?: (evt: Event) => void): MonoTypeOperatorFunction { +export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event) => void): + MonoTypeOperatorFunction { return function(source: Observable) { return source.pipe(mergeMap(t => { - const {targetSnapshot, currentSnapshot} = t; - const checks = getAllRouteGuards(targetSnapshot !, currentSnapshot, rootContexts); - if (checks.canDeactivateChecks.length === 0 && checks.canActivateChecks.length === 0) { + const {targetSnapshot, currentSnapshot, guards: {canActivateChecks, canDeactivateChecks}} = t; + if (canDeactivateChecks.length === 0 && canActivateChecks.length === 0) { return of ({...t, guardsResult: true}); } - return runCanDeactivateChecks(checks, targetSnapshot !, currentSnapshot, moduleInjector) + return runCanDeactivateChecks( + canDeactivateChecks, targetSnapshot !, currentSnapshot, moduleInjector) .pipe( mergeMap((canDeactivate: boolean) => { return canDeactivate ? - runCanActivateChecks(targetSnapshot !, checks, moduleInjector, forwardEvent) : + runCanActivateChecks( + targetSnapshot !, canActivateChecks, moduleInjector, forwardEvent) : of (false); }), map(guardsResult => ({...t, guardsResult}))); @@ -42,28 +41,26 @@ export function checkGuards( } function runCanDeactivateChecks( - checks: Checks, futureRSS: RouterStateSnapshot, currRSS: RouterStateSnapshot, + checks: CanDeactivate[], futureRSS: RouterStateSnapshot, currRSS: RouterStateSnapshot, moduleInjector: Injector): Observable { - return from(checks.canDeactivateChecks) - .pipe( - mergeMap( - (check: CanDeactivate) => runCanDeactivate( - check.component, check.route, currRSS, futureRSS, moduleInjector)), - every((result: boolean) => result === true)); + return from(checks).pipe( + mergeMap( + (check: CanDeactivate) => + runCanDeactivate(check.component, check.route, currRSS, futureRSS, moduleInjector)), + every((result: boolean) => result === true)); } function runCanActivateChecks( - futureSnapshot: RouterStateSnapshot, checks: Checks, moduleInjector: Injector, + futureSnapshot: RouterStateSnapshot, checks: CanActivate[], moduleInjector: Injector, forwardEvent?: (evt: Event) => void): Observable { - return from(checks.canActivateChecks) - .pipe( - concatMap((check: CanActivate) => andObservables(from([ - fireChildActivationStart(check.route.parent, forwardEvent), - fireActivationStart(check.route, forwardEvent), - runCanActivateChild(futureSnapshot, check.path, moduleInjector), - runCanActivate(futureSnapshot, check.route, moduleInjector) - ]))), - every((result: boolean) => result === true)); + return from(checks).pipe( + concatMap((check: CanActivate) => andObservables(from([ + fireChildActivationStart(check.route.parent, forwardEvent), + fireActivationStart(check.route, forwardEvent), + runCanActivateChild(futureSnapshot, check.path, moduleInjector), + runCanActivate(futureSnapshot, check.route, moduleInjector) + ]))), + every((result: boolean) => result === true)); } /** diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts index ff633d755f..e74737a7cf 100644 --- a/packages/router/src/operators/resolve_data.ts +++ b/packages/router/src/operators/resolve_data.ts @@ -12,25 +12,23 @@ import {concatMap, last, map, mergeMap, reduce} from 'rxjs/operators'; import {ResolveData} from '../config'; import {NavigationTransition} from '../router'; -import {ChildrenOutletContexts} from '../router_outlet_context'; import {ActivatedRouteSnapshot, RouterStateSnapshot, inheritedParamsDataResolve} from '../router_state'; import {wrapIntoObservable} from '../utils/collection'; -import {getAllRouteGuards, getToken} from '../utils/preactivation'; +import {getToken} from '../utils/preactivation'; export function resolveData( - rootContexts: ChildrenOutletContexts, paramsInheritanceStrategy: 'emptyOnly' | 'always', + paramsInheritanceStrategy: 'emptyOnly' | 'always', moduleInjector: Injector): MonoTypeOperatorFunction { return function(source: Observable) { return source.pipe(mergeMap(t => { - const {targetSnapshot, currentSnapshot} = t; - const checks = getAllRouteGuards(targetSnapshot !, currentSnapshot, rootContexts); + const {targetSnapshot, guards: {canActivateChecks}} = t; - if (!checks.canActivateChecks.length) { + if (!canActivateChecks.length) { return of (t); } - return from(checks.canActivateChecks) + return from(canActivateChecks) .pipe( concatMap( check => runResolve( diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 211813b5dd..57febc1227 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -28,7 +28,7 @@ import {ActivatedRoute, RouterState, RouterStateSnapshot, createEmptyState} from import {Params, isNavigationCancelingError} from './shared'; import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy'; import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tree'; -import {getAllRouteGuards} from './utils/preactivation'; +import {Checks, getAllRouteGuards} from './utils/preactivation'; @@ -185,6 +185,7 @@ export type NavigationTransition = { targetSnapshot: RouterStateSnapshot | null, currentRouterState: RouterState, targetRouterState: RouterState | null, + guards: Checks, guardsResult: boolean | null, }; @@ -354,6 +355,7 @@ export class Router { targetSnapshot: null, currentRouterState: this.routerState, targetRouterState: null, + guards: {canActivateChecks: [], canDeactivateChecks: []}, guardsResult: null, }); this.navigations = this.setupNavigations(this.transitions); @@ -478,9 +480,13 @@ export class Router { this.triggerEvent(guardsStart); }), - checkGuards( - this.rootContexts, this.ngModule.injector, - (evt: Event) => this.triggerEvent(evt)), + map(t => ({ + ...t, + guards: + getAllRouteGuards(t.targetSnapshot !, t.currentSnapshot, this.rootContexts) + })), + + checkGuards(this.ngModule.injector, (evt: Event) => this.triggerEvent(evt)), tap(t => { const guardsEnd = new GuardsCheckEnd( @@ -503,8 +509,7 @@ export class Router { // --- RESOLVE --- switchTap(t => { - if (getAllRouteGuards(t.targetSnapshot !, t.currentSnapshot, this.rootContexts) - .canActivateChecks.length) { + if (t.guards.canActivateChecks.length) { return of (t).pipe( tap(t => { const resolveStart = new ResolveStart( @@ -513,7 +518,7 @@ export class Router { this.triggerEvent(resolveStart); }), resolveData( - this.rootContexts, this.paramsInheritanceStrategy, + this.paramsInheritanceStrategy, this.ngModule.injector), // tap(t => { const resolveEnd = new ResolveEnd( diff --git a/packages/router/test/router.spec.ts b/packages/router/test/router.spec.ts index 2524811c62..c39c39e0a8 100644 --- a/packages/router/test/router.spec.ts +++ b/packages/router/test/router.spec.ts @@ -18,6 +18,7 @@ import {NavigationTransition, Router} from '../src/router'; import {ChildrenOutletContexts} from '../src/router_outlet_context'; import {RouterStateSnapshot, createEmptyStateSnapshot} from '../src/router_state'; import {DefaultUrlSerializer} from '../src/url_tree'; +import {getAllRouteGuards} from '../src/utils/preactivation'; import {TreeNode} from '../src/utils/tree'; import {RouterTestingModule} from '../testing/src/router_testing_module'; @@ -143,9 +144,8 @@ describe('Router', () => { const futureState = new (RouterStateSnapshot as any)( 'url', new TreeNode(empty.root, [new TreeNode(childSnapshot, [])])); - of ({targetSnapshot: futureState, currentSnapshot: empty}) - .pipe(checkGuardsOperator( - new ChildrenOutletContexts(), TestBed, (evt) => { events.push(evt); })) + of ({guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts())}) + .pipe(checkGuardsOperator(TestBed, (evt) => { events.push(evt); })) .subscribe((x) => result = !!x.guardsResult, (e) => { throw e; }); expect(result).toBe(true); @@ -178,9 +178,8 @@ describe('Router', () => { new TreeNode(grandchildSnapshot, [new TreeNode(greatGrandchildSnapshot, [])]) ])])); - of ({targetSnapshot: futureState, currentSnapshot: empty}) - .pipe(checkGuardsOperator( - new ChildrenOutletContexts(), TestBed, (evt) => { events.push(evt); })) + of ({guards: getAllRouteGuards(futureState, empty, new ChildrenOutletContexts())}) + .pipe(checkGuardsOperator(TestBed, (evt) => { events.push(evt); })) .subscribe((x) => result = !!x.guardsResult, (e) => { throw e; }); expect(result).toBe(true); @@ -211,9 +210,8 @@ describe('Router', () => { new TreeNode( empty.root, [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])])); - of ({targetSnapshot: futureState, currentSnapshot: currentState}) - .pipe(checkGuardsOperator( - new ChildrenOutletContexts(), TestBed, (evt) => { events.push(evt); })) + of ({guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts())}) + .pipe(checkGuardsOperator(TestBed, (evt) => { events.push(evt); })) .subscribe((x) => result = !!x.guardsResult, (e) => { throw e; }); expect(result).toBe(true); @@ -257,9 +255,8 @@ describe('Router', () => { greatGrandchildSnapshot, [new TreeNode(greatGreatGrandchildSnapshot, [])]) ])])])); - of ({targetSnapshot: futureState, currentSnapshot: currentState}) - .pipe(checkGuardsOperator( - new ChildrenOutletContexts(), TestBed, (evt) => { events.push(evt); })) + of ({guards: getAllRouteGuards(futureState, currentState, new ChildrenOutletContexts())}) + .pipe(checkGuardsOperator(TestBed, (evt) => { events.push(evt); })) .subscribe((x) => result = !!x.guardsResult, (e) => { throw e; }); expect(result).toBe(true); @@ -538,15 +535,19 @@ describe('Router', () => { function checkResolveData( future: RouterStateSnapshot, curr: RouterStateSnapshot, injector: any, check: any): void { - of ({ targetSnapshot: future, currentSnapshot: curr } as Partial) - .pipe(resolveDataOperator(new ChildrenOutletContexts(), 'emptyOnly', injector)) + of ({ + guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()) + } as Partial) + .pipe(resolveDataOperator('emptyOnly', injector)) .subscribe(check, (e) => { throw e; }); } function checkGuards( future: RouterStateSnapshot, curr: RouterStateSnapshot, injector: any, check: (result: boolean) => void): void { - of ({ targetSnapshot: future, currentSnapshot: curr } as Partial) - .pipe(checkGuardsOperator(new ChildrenOutletContexts(), injector)) + of ({ + guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()) + } as Partial) + .pipe(checkGuardsOperator(injector)) .subscribe(t => check(!!t.guardsResult), (e) => { throw e; }); }