fix(router): resolve guard observables on the first emit (angular#10412)

matsko · Nov 3, 2016 · 2e78b76 · 2e78b76
1 parent b2cf379
commit 2e78b76
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts
@@ -17,8 +17,8 @@ import {fromPromise} from 'rxjs/observable/fromPromise';
 import {of } from 'rxjs/observable/of';
 import {concatMap} from 'rxjs/operator/concatMap';
 import {every} from 'rxjs/operator/every';
+import {first} from 'rxjs/operator/first';
 import {map} from 'rxjs/operator/map';
-import {mergeAll} from 'rxjs/operator/mergeAll';
 import {mergeMap} from 'rxjs/operator/mergeMap';
 import {reduce} from 'rxjs/operator/reduce';
 
@@ -783,7 +783,7 @@ export class PreActivation {
   checkGuards(): Observable<boolean> {
     if (this.checks.length === 0) return of (true);
     const checks$ = from(this.checks);
-    const runningChecks$ = map.call(checks$, (s: any) => {
+    const runningChecks$ = mergeMap.call(checks$, (s: any) => {
       if (s instanceof CanActivate) {
         return andObservables(
             from([this.runCanActivateChild(s.path), this.runCanActivate(s.route)]));
@@ -795,8 +795,7 @@ export class PreActivation {
         throw new Error('Cannot be reached');
       }
     });
-    const mergedChecks$ = mergeAll.call(runningChecks$);
-    return every.call(mergedChecks$, (result: any) => result === true);
+    return every.call(runningChecks$, (result: any) => result === true);
   }
 
   resolveData(): Observable<any> {
@@ -898,11 +897,13 @@ export class PreActivation {
     if (!canActivate || canActivate.length === 0) return of (true);
     const obs = map.call(from(canActivate), (c: any) => {
       const guard = this.getToken(c, future);
+      let observable: Observable<boolean>;
       if (guard.canActivate) {
-        return wrapIntoObservable(guard.canActivate(future, this.future));
+        observable = wrapIntoObservable(guard.canActivate(future, this.future));
       } else {
-        return wrapIntoObservable(guard(future, this.future));
+        observable = wrapIntoObservable(guard(future, this.future));
       }
+      return first.call(observable);
     });
     return andObservables(obs);
   }
@@ -918,11 +919,13 @@ export class PreActivation {
     return andObservables(map.call(from(canActivateChildGuards), (d: any) => {
       const obs = map.call(from(d.guards), (c: any) => {
         const guard = this.getToken(c, c.node);
+        let observable: Observable<boolean>;
         if (guard.canActivateChild) {
-          return wrapIntoObservable(guard.canActivateChild(future, this.future));
+          observable = wrapIntoObservable(guard.canActivateChild(future, this.future));
         } else {
-          return wrapIntoObservable(guard(future, this.future));
+          observable = wrapIntoObservable(guard(future, this.future));
         }
+        return first.call(observable);
       });
       return andObservables(obs);
     }));
@@ -938,16 +941,17 @@ export class PreActivation {
   private runCanDeactivate(component: Object, curr: ActivatedRouteSnapshot): Observable<boolean> {
     const canDeactivate = curr && curr._routeConfig ? curr._routeConfig.canDeactivate : null;
     if (!canDeactivate || canDeactivate.length === 0) return of (true);
-    const canDeactivate$ = map.call(from(canDeactivate), (c: any) => {
+    const canDeactivate$ = mergeMap.call(from(canDeactivate), (c: any) => {
       const guard = this.getToken(c, curr);
+      let observable: Observable<boolean>;
       if (guard.canDeactivate) {
-        return wrapIntoObservable(guard.canDeactivate(component, curr, this.curr));
+        observable = wrapIntoObservable(guard.canDeactivate(component, curr, this.curr));
       } else {
-        return wrapIntoObservable(guard(component, curr, this.curr));
+        observable = wrapIntoObservable(guard(component, curr, this.curr));
       }
+      return first.call(observable);
     });
-    const merged$ = mergeAll.call(canDeactivate$);
-    return every.call(merged$, (result: any) => result === true);
+    return every.call(canDeactivate$, (result: any) => result === true);
   }
 
   private runResolve(future: ActivatedRouteSnapshot): Observable<any> {

diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts
@@ -11,7 +11,6 @@ import {Component, NgModule, NgModuleFactoryLoader} from '@angular/core';
 import {ComponentFixture, TestBed, async, fakeAsync, inject, tick} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/matchers';
 import {Observable} from 'rxjs/Observable';
-import {of } from 'rxjs/observable/of';
 import {map} from 'rxjs/operator/map';
 
 import {ActivatedRoute, ActivatedRouteSnapshot, CanActivate, CanDeactivate, Event, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, Params, PreloadAllModules, PreloadingStrategy, Resolve, Router, RouterModule, RouterStateSnapshot, RoutesRecognized, UrlHandlingStrategy, UrlSegmentGroup, UrlTree} from '../index';
@@ -1163,7 +1162,9 @@ describe('Integration', () => {
           TestBed.configureTestingModule({
             providers: [{
               provide: 'CanActivate',
-              useValue: (a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => of (false),
+              useValue: (a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
+                return Observable.create((observer: any) => { observer.next(false); });
+              }
             }]
           });
         });
@@ -1438,7 +1439,7 @@ describe('Integration', () => {
             providers: [{
               provide: 'CanDeactivate',
               useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
-                return of (false);
+                return Observable.create((observer: any) => { observer.next(false); });
               }
             }]
           });