feat(router): allow guards to return UrlTree as well as boolean (angular#26521)

* Removed `andObservable` helper function in favor of inline implementation
* Flow `boolean | UrlTree` through guards check
* Add tests to verify behavior of `checkGuards` function flowing `UrlTree` properly

PR Close angular#26521

Author: jasonaden

packages/router/src/apply_redirects.ts

@@ -8,14 +8,14 @@
 
 import {Injector, NgModuleRef} from '@angular/core';
 import {EmptyError, Observable, Observer, from, of } from 'rxjs';
-import {catchError, concatAll, first, map, mergeMap} from 'rxjs/operators';
+import {catchError, concatAll, every, first, map, mergeMap} from 'rxjs/operators';
 
 import {LoadedRouterConfig, Route, Routes} from './config';
 import {CanLoadFn} from './interfaces';
 import {RouterConfigLoader} from './router_config_loader';
 import {PRIMARY_OUTLET, Params, defaultUrlMatcher, navigationCancelingError} from './shared';
 import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
-import {andObservables, forEach, waitForMap, wrapIntoObservable} from './utils/collection';
+import {forEach, waitForMap, wrapIntoObservable} from './utils/collection';
 import {isCanLoad, isFunction} from './utils/type_guards';
 
 class NoMatch {
@@ -421,7 +421,7 @@ function runCanLoadGuard(
     return wrapIntoObservable(guardVal);
   }));
 
-  return andObservables(obs);
+  return obs.pipe(concatAll(), every(result => result === true));
 }
 
 function match(segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[]): {

packages/router/src/operators/check_guards.ts

@@ -7,17 +7,19 @@
  */
 
 import {Injector} from '@angular/core';
-import {MonoTypeOperatorFunction, Observable, from, of } from 'rxjs';
-import {concatMap, every, first, map, mergeMap} from 'rxjs/operators';
+import {MonoTypeOperatorFunction, Observable, defer, from, of } from 'rxjs';
+import {concatAll, concatMap, first, map, mergeMap} from 'rxjs/operators';
 
 import {ActivationStart, ChildActivationStart, Event} from '../events';
 import {CanActivateChildFn, CanActivateFn, CanDeactivateFn} from '../interfaces';
 import {NavigationTransition} from '../router';
 import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state';
 import {UrlTree} from '../url_tree';
-import {andObservables, wrapIntoObservable} from '../utils/collection';
+import {wrapIntoObservable} from '../utils/collection';
 import {CanActivate, CanDeactivate, getCanActivateChild, getToken} from '../utils/preactivation';
-import {isCanActivate, isCanActivateChild, isCanDeactivate, isFunction} from '../utils/type_guards';
+import {isCanActivate, isCanActivateChild, isCanDeactivate, isFunction, isBoolean} from '../utils/type_guards';
+
+import {prioritizedGuardValue} from './prioritized_guard_value';
 
 export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event) => void):
     MonoTypeOperatorFunction<NavigationTransition> {
@@ -33,10 +35,10 @@ export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event
                  canDeactivateChecks, targetSnapshot !, currentSnapshot, moduleInjector)
           .pipe(
               mergeMap(canDeactivate => {
-                return canDeactivate ?
+                return canDeactivate && isBoolean(canDeactivate) ?
                     runCanActivateChecks(
                         targetSnapshot !, canActivateChecks, moduleInjector, forwardEvent) :
-                    of (false);
+                    of (canDeactivate);
               }),
               map(guardsResult => ({...t, guardsResult})));
     }));
@@ -45,25 +47,30 @@ export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event
 
 function runCanDeactivateChecks(
     checks: CanDeactivate[], futureRSS: RouterStateSnapshot, currRSS: RouterStateSnapshot,
-    moduleInjector: Injector): Observable<boolean|UrlTree> {
+    moduleInjector: Injector) {
   return from(checks).pipe(
-      mergeMap(check => {
-        return runCanDeactivate(check.component, check.route, currRSS, futureRSS, moduleInjector);
-      }),
-      every(result => result === true));
+      mergeMap(
+          check =>
+              runCanDeactivate(check.component, check.route, currRSS, futureRSS, moduleInjector)),
+      first(result => { return result !== true; }, true as boolean | UrlTree));
 }
 
 function runCanActivateChecks(
     futureSnapshot: RouterStateSnapshot, checks: CanActivate[], moduleInjector: Injector,
-    forwardEvent?: (evt: Event) => void): Observable<boolean> {
+    forwardEvent?: (evt: Event) => void) {
   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));
+      concatMap((check: CanActivate) => {
+        return from([
+                 fireChildActivationStart(check.route.parent, forwardEvent),
+                 fireActivationStart(check.route, forwardEvent),
+                 runCanActivateChild(futureSnapshot, check.path, moduleInjector),
+                 runCanActivate(futureSnapshot, check.route, moduleInjector)
+               ])
+            .pipe(concatAll(), first(result => {
+                    return result !== true;
+                  }, true as boolean | UrlTree));
+      }),
+      first(result => { return result !== true; }, true as boolean | UrlTree));
 }
 
 /**
@@ -102,57 +109,63 @@ function fireChildActivationStart(
 
 function runCanActivate(
     futureRSS: RouterStateSnapshot, futureARS: ActivatedRouteSnapshot,
-    moduleInjector: Injector): Observable<boolean> {
+    moduleInjector: Injector): Observable<boolean|UrlTree> {
   const canActivate = futureARS.routeConfig ? futureARS.routeConfig.canActivate : null;
   if (!canActivate || canActivate.length === 0) return of (true);
-  const obs = from(canActivate).pipe(map((c: any) => {
-    const guard = getToken(c, futureARS, moduleInjector);
-    let observable;
-    if (isCanActivate(guard)) {
-      observable = wrapIntoObservable(guard.canActivate(futureARS, futureRSS));
-    } else if (isFunction<CanActivateFn>(guard)) {
-      observable = wrapIntoObservable(guard(futureARS, futureRSS));
-    } else {
-      throw new Error('Invalid canActivate guard');
-    }
-    return observable.pipe(first());
-  }));
-  return andObservables(obs);
+
+  const canActivateObservables = canActivate.map((c: any) => {
+    return defer(() => {
+      const guard = getToken(c, futureARS, moduleInjector);
+      let observable;
+      if (isCanActivate(guard)) {
+        observable = wrapIntoObservable(guard.canActivate(futureARS, futureRSS));
+      } else if (isFunction<CanActivateFn>(guard)) {
+        observable = wrapIntoObservable(guard(futureARS, futureRSS));
+      } else {
+        throw new Error('Invalid CanActivate guard');
+      }
+      return observable.pipe(first());
+    });
+  });
+  return of (canActivateObservables).pipe(prioritizedGuardValue());
 }
 
 function runCanActivateChild(
     futureRSS: RouterStateSnapshot, path: ActivatedRouteSnapshot[],
-    moduleInjector: Injector): Observable<boolean> {
+    moduleInjector: Injector): Observable<boolean|UrlTree> {
   const futureARS = path[path.length - 1];
 
   const canActivateChildGuards = path.slice(0, path.length - 1)
                                      .reverse()
                                      .map(p => getCanActivateChild(p))
                                      .filter(_ => _ !== null);
 
-  return andObservables(from(canActivateChildGuards).pipe(map((d: any) => {
-    const obs = from(d.guards).pipe(map((c: any) => {
-      const guard = getToken(c, d.node, moduleInjector);
-      let observable;
-      if (isCanActivateChild(guard)) {
-        observable = wrapIntoObservable(guard.canActivateChild(futureARS, futureRSS));
-      } else if (isFunction<CanActivateChildFn>(guard)) {
-        observable = wrapIntoObservable(guard(futureARS, futureRSS));
-      } else {
-        throw new Error('Invalid CanActivateChild guard');
-      }
-      return observable.pipe(first());
-    }));
-    return andObservables(obs);
-  })));
+  const canActivateChildGuardsMapped = canActivateChildGuards.map((d: any) => {
+    return defer(() => {
+      const guardsMapped = d.guards.map((c: any) => {
+        const guard = getToken(c, d.node, moduleInjector);
+        let observable;
+        if (isCanActivateChild(guard)) {
+          observable = wrapIntoObservable(guard.canActivateChild(futureARS, futureRSS));
+        } else if (isFunction<CanActivateChildFn>(guard)) {
+          observable = wrapIntoObservable(guard(futureARS, futureRSS));
+        } else {
+          throw new Error('Invalid CanActivateChild guard');
+        }
+        return observable.pipe(first());
+      });
+      return of (guardsMapped).pipe(prioritizedGuardValue());
+    });
+  });
+  return of (canActivateChildGuardsMapped).pipe(prioritizedGuardValue());
 }
 
 function runCanDeactivate(
-    component: Object| null, currARS: ActivatedRouteSnapshot, currRSS: RouterStateSnapshot,
+    component: Object | null, currARS: ActivatedRouteSnapshot, currRSS: RouterStateSnapshot,
     futureRSS: RouterStateSnapshot, moduleInjector: Injector): Observable<boolean|UrlTree> {
   const canDeactivate = currARS && currARS.routeConfig ? currARS.routeConfig.canDeactivate : null;
   if (!canDeactivate || canDeactivate.length === 0) return of (true);
-  const canDeactivate$ = from(canDeactivate).pipe(mergeMap((c: any) => {
+  const canDeactivateObservables = canDeactivate.map((c: any) => {
     const guard = getToken(c, currARS, moduleInjector);
     let observable;
     if (isCanDeactivate(guard)) {
@@ -164,6 +177,6 @@ function runCanDeactivate(
       throw new Error('Invalid CanDeactivate guard');
     }
     return observable.pipe(first());
-  }));
-  return canDeactivate$.pipe(every((result: any) => result === true));
+  });
+  return of (canDeactivateObservables).pipe(prioritizedGuardValue());
 }

packages/router/src/router.ts

@@ -186,7 +186,7 @@ export type NavigationTransition = {
   currentRouterState: RouterState,
   targetRouterState: RouterState | null,
   guards: Checks,
-  guardsResult: boolean | null,
+  guardsResult: boolean | UrlTree | null,
 };
 
 /**

packages/router/src/utils/collection.ts

@@ -8,7 +8,7 @@
 
 import {NgModuleFactory, ɵisObservable as isObservable, ɵisPromise as isPromise} from '@angular/core';
 import {Observable, from, of } from 'rxjs';
-import {concatAll, every, last as lastValue, map, mergeAll} from 'rxjs/operators';
+import {concatAll, last as lastValue, map} from 'rxjs/operators';
 
 import {PRIMARY_OUTLET} from '../shared';
 
@@ -88,14 +88,6 @@ export function waitForMap<A, B>(
   return of .apply(null, waitHead.concat(waitTail)).pipe(concatAll(), lastValue(), map(() => res));
 }
 
-/**
- * ANDs Observables by merging all input observables, reducing to an Observable verifying all
- * input Observables return `true`.
- */
-export function andObservables(observables: Observable<Observable<any>>): Observable<boolean> {
-  return observables.pipe(mergeAll(), every((result: any) => result === true));
-}
-
 export function wrapIntoObservable<T>(value: T | NgModuleFactory<T>| Promise<T>| Observable<T>) {
   if (isObservable(value)) {
     return value;

packages/router/src/utils/type_guards.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Type} from '@angular/core';
 import {CanActivate, CanActivateChild, CanDeactivate, CanLoad} from '../interfaces';
 
 /**
@@ -26,6 +25,10 @@ export function isFunction<T>(v: any): v is T {
   return typeof v === 'function';
 }
 
+export function isBoolean(v: any): v is boolean {
+  return typeof v === 'boolean';
+}
+
 export function isCanLoad(guard: any): guard is CanLoad {
   return guard && isFunction<CanLoad>(guard.canLoad);
 }

packages/router/test/helpers.ts

@@ -19,7 +19,7 @@ export class Logger {
   empty() { this.logs.length = 0; }
 }
 
-export function provideTokenLogger(token: string, returnValue = true) {
+export function provideTokenLogger(token: string, returnValue = true as boolean | UrlTree) {
   return {
     provide: token,
     useFactory: (logger: Logger) => () => (logger.add(token), returnValue),