fixup! feat(router): guard returning UrlTree cancels current navigati…

…on and redirects
angular · Oct 17, 2018 · a1f41fc · a1f41fc
1 parent ef48f54
commit a1f41fc
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 13 deletions.
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -8,7 +8,7 @@
 
 import {Location} from '@angular/common';
 import {Compiler, Injector, NgModuleFactoryLoader, NgModuleRef, NgZone, Type, isDevMode, ɵConsole as Console} from '@angular/core';
-import {BehaviorSubject, EMPTY, Observable, Subject, Subscription, of } from 'rxjs';
+import {BehaviorSubject, EMPTY, Observable, Subject, Subscription, defer, of } from 'rxjs';
 import {catchError, filter, finalize, map, switchMap, tap} from 'rxjs/operators';
 
 import {QueryParamsHandling, Route, Routes, standardizeConfig, validateConfig} from './config';
@@ -490,7 +490,8 @@ export class Router {
               checkGuards(this.ngModule.injector, (evt: Event) => this.triggerEvent(evt)),
               tap(t => {
                 if (isUrlTree(t.guardsResult)) {
-                  const error: Error&{url?: UrlTree} = navigationCancelingError('');
+                  const error: Error&{url?: UrlTree} = navigationCancelingError(
+                      `Redirecting to "${this.serializeUrl(t.guardsResult)}"`);
                   error.url = t.guardsResult;
                   throw error;
                 }
@@ -610,15 +611,19 @@ export class Router {
                  * rather than an error. */
                 if (isNavigationCancelingError(e)) {
                   this.navigated = true;
-                  if (isUrlTree(e.url)) {
-                    this.navigate(e.url);
-                  } else {
+                  const redirecting = isUrlTree(e.url);
+                  if (!redirecting) {
                     this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl);
                   }
                   const navCancel =
                       new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), e.message);
                   eventsSubject.next(navCancel);
                   t.resolve(false);
+
+                  if (redirecting) {
+                    this.navigateByUrl(e.url);
+                  }
+
                   /* All other errors should reset to the router's internal URL reference to the
                    * pre-error state. */
                 } else {

diff --git a/packages/router/src/utils/type_guards.ts b/packages/router/src/utils/type_guards.ts
@@ -43,7 +43,7 @@ export function isCanActivate(guard: any): guard is CanActivate {
 }
 
 export function isCanActivateChild(guard: any): guard is CanActivateChild {
-  return guard && isFunction<CanActivateChild>(guard.canActivate);
+  return guard && isFunction<CanActivateChild>(guard.canActivateChild);
 }
 
 export function isCanDeactivate<T>(guard: any): guard is CanDeactivate<T> {

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -9,7 +9,7 @@
 import {CommonModule, Location} from '@angular/common';
 import {SpyLocation} from '@angular/common/testing';
 import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, OnDestroy, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core';
-import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/testing';
+import {ComponentFixture, TestBed, fakeAsync, flush, inject, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router';
@@ -2017,6 +2017,59 @@ describe('Integration', () => {
            })));
       });
 
+      describe('should redirect when guard returns UrlTree', () => {
+        beforeEach(() => TestBed.configureTestingModule({
+          providers: [{
+            provide: 'returnUrlTree',
+            useFactory: (router: Router) => () => { return router.parseUrl('/redirected'); },
+            deps: [Router]
+          }]
+        }));
+
+        it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
+             const recordedEvents: any[] = [];
+             let cancelEvent: NavigationCancel = null !;
+             router.events.forEach((e: any) => {
+               recordedEvents.push(e);
+               if (e instanceof NavigationCancel) cancelEvent = e;
+             });
+             router.resetConfig([
+               {path: '', component: SimpleCmp},
+               {path: 'one', component: RouteCmp, canActivate: ['returnUrlTree']},
+               {path: 'redirected', component: SimpleCmp}
+             ]);
+
+             const fixture = TestBed.createComponent(RootCmp);
+             router.navigateByUrl('/one');
+
+             advance(fixture);
+
+             expect(location.path()).toEqual('/redirected');
+             expect(fixture.nativeElement).toHaveText('simple');
+             expect(cancelEvent && cancelEvent.reason)
+                 .toBe('NavigationCancelingError: Redirecting to "/redirected"');
+             expectEvents(recordedEvents, [
+               [NavigationStart, '/one'],
+               [RoutesRecognized, '/one'],
+               [GuardsCheckStart, '/one'],
+               [ChildActivationStart, undefined],
+               [ActivationStart, undefined],
+               [NavigationCancel, '/one'],
+               [NavigationStart, '/redirected'],
+               [RoutesRecognized, '/redirected'],
+               [GuardsCheckStart, '/redirected'],
+               [ChildActivationStart, undefined],
+               [ActivationStart, undefined],
+               [GuardsCheckEnd, '/redirected'],
+               [ResolveStart, '/redirected'],
+               [ResolveEnd, '/redirected'],
+               [ActivationEnd, undefined],
+               [ChildActivationEnd, undefined],
+               [NavigationEnd, '/redirected'],
+             ]);
+           })));
+      });
+
       describe('runGuardsAndResolvers', () => {
         let guardRunCount = 0;
         let resolverRunCount = 0;

diff --git a/packages/router/test/router.spec.ts b/packages/router/test/router.spec.ts
@@ -672,10 +672,11 @@ function checkGuards(
     guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts())
   } as Partial<NavigationTransition>)
       .pipe(checkGuardsOperator(injector))
-      .subscribe(
-          t => {
-            if (t.guardsResult === null) throw new Error('Guard result expected');
-            return check(t.guardsResult);
-          },
-          (e) => { throw e; });
+      .subscribe({
+        next(t) {
+          if (t.guardsResult === null) throw new Error('Guard result expected');
+          return check(t.guardsResult);
+        },
+        error(e) { throw e; }
+      });
 }