refactor(router): get guards only one time and simplify guard operator signature (angular#26239)

jasonaden · jasonaden · commit 532e53678d42 · 2018-10-04T10:10:28.000-07:00
PR Close angular#26239
diff --git 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<NavigationTransition> {
+export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event) => void):
+    MonoTypeOperatorFunction<NavigationTransition> {
   return function(source: Observable<NavigationTransition>) {
 
     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<boolean> {
-  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<boolean> {
-  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
@@ -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<NavigationTransition> {
   return function(source: Observable<NavigationTransition>) {
     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
@@ -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
@@ -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<NavigationTransition>)
-      .pipe(resolveDataOperator(new ChildrenOutletContexts(), 'emptyOnly', injector))
+  of ({
+    guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts())
+  } as Partial<NavigationTransition>)
+      .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<NavigationTransition>)
-      .pipe(checkGuardsOperator(new ChildrenOutletContexts(), injector))
+  of ({
+    guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts())
+  } as Partial<NavigationTransition>)
+      .pipe(checkGuardsOperator(injector))
       .subscribe(t => check(!!t.guardsResult), (e) => { throw e; });
 }
