Skip to content
Permalink
Browse files

fix(router): ensure URL is updated after second redirect with UrlUpda…

…teStrategy="eager" (#27523)

Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes #27116

PR Close #27523
  • Loading branch information...
jasonaden authored and mhevery committed Dec 6, 2018
1 parent fdf3998 commit ad26cd6d0cae093beb4d936609e9d805b3528505
Showing with 110 additions and 50 deletions.
  1. +10 −3 packages/router/src/router.ts
  2. +100 −47 packages/router/test/integration.spec.ts
@@ -291,6 +291,7 @@ function defaultRouterHook(snapshot: RouterStateSnapshot, runExtras: {
export class Router {
private currentUrlTree: UrlTree;
private rawUrlTree: UrlTree;
private browserUrlTree: UrlTree;
private readonly transitions: BehaviorSubject<NavigationTransition>;
private navigations: Observable<NavigationTransition>;
private lastSuccessfulNavigation: Navigation|null = null;
@@ -400,6 +401,7 @@ export class Router {
this.resetConfig(config);
this.currentUrlTree = createEmptyUrlTree();
this.rawUrlTree = this.currentUrlTree;
this.browserUrlTree = this.parseUrl(this.location.path());

this.configLoader = new RouterConfigLoader(loader, compiler, onLoadStart, onLoadEnd);
this.routerState = createEmptyState(this.currentUrlTree, this.rootComponentType);
@@ -461,7 +463,7 @@ export class Router {
return of (t).pipe(
switchMap(t => {
const urlTransition =
!this.navigated || t.extractedUrl.toString() !== this.currentUrlTree.toString();
!this.navigated || t.extractedUrl.toString() !== this.browserUrlTree.toString();
const processCurrentUrl =
(this.onSameUrlNavigation === 'reload' ? true : urlTransition) &&
this.urlHandlingStrategy.shouldProcessUrl(t.rawUrl);
@@ -502,8 +504,12 @@ export class Router {
this.paramsInheritanceStrategy, this.relativeLinkResolution),

// Update URL if in `eager` update mode
tap(t => this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange &&
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id)),
tap(t => {
if (this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange) {
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id);
this.browserUrlTree = t.urlAfterRedirects;
}
}),

// Fire RoutesRecognized
tap(t => {
@@ -665,6 +671,7 @@ export class Router {

if (this.urlUpdateStrategy === 'deferred' && !t.extras.skipLocationChange) {
this.setBrowserUrl(this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
this.browserUrlTree = t.urlAfterRedirects;
}
}),

@@ -13,7 +13,7 @@ import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {fixmeIvy} from '@angular/private/testing';
import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, Navigation, 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';
import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DefaultUrlSerializer, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, Navigation, 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';
import {Observable, Observer, Subscription, of } from 'rxjs';
import {filter, first, map, tap} from 'rxjs/operators';

@@ -575,64 +575,110 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));

it('should eagerly update the URL with urlUpdateStrategy="eagar"',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp);
advance(fixture);
describe('"eager" urlUpdateStrategy', () => {
beforeEach(() => {
const serializer = new DefaultUrlSerializer();
TestBed.configureTestingModule({
providers: [{
provide: 'authGuardFail',
useValue: (a: any, b: any) => {
return new Promise(res => { setTimeout(() => res(serializer.parse('/login')), 1); });
}
}]
});

router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
});

router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');

expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
it('should eagerly update the URL with urlUpdateStrategy="eagar"',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp);
advance(fixture);

router.resetConfig([{path: 'team/:id', component: TeamCmp}]);

router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');

router.urlUpdateStrategy = 'eager';
(router as any).hooks.beforePreactivation = () => {
expect(location.path()).toEqual('/team/33');
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
return of (null);
};
router.navigateByUrl('/team/33');

advance(fixture);
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));
router.urlUpdateStrategy = 'eager';
(router as any).hooks.beforePreactivation = () => {
expect(location.path()).toEqual('/team/33');
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
return of (null);
};
router.navigateByUrl('/team/33');

it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eagar"',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp);
advance(fixture);
advance(fixture);
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));
it('should eagerly update the URL with urlUpdateStrategy="eagar"',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp);
advance(fixture);

router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
router.urlUpdateStrategy = 'eager';

router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');
router.resetConfig([
{path: 'team/:id', component: SimpleCmp, canActivate: ['authGuardFail']},
{path: 'login', component: AbsoluteSimpleLinkCmp}
]);

expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');

router.urlUpdateStrategy = 'eager';
// Redirects to /login
advance(fixture, 1);
expect(location.path()).toEqual('/login');

let urlAtNavStart = '';
let urlAtRoutesRecognized = '';
router.events.subscribe(e => {
if (e instanceof NavigationStart) {
urlAtNavStart = location.path();
}
if (e instanceof RoutesRecognized) {
urlAtRoutesRecognized = location.path();
}
});
// Perform the same logic again, and it should produce the same result
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');

router.navigateByUrl('/team/33');
// Redirects to /login
advance(fixture, 1);
expect(location.path()).toEqual('/login');
})));

advance(fixture);
expect(urlAtNavStart).toBe('/team/22');
expect(urlAtRoutesRecognized).toBe('/team/33');
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));
it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eagar"',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp);
advance(fixture);

router.resetConfig([{path: 'team/:id', component: TeamCmp}]);

router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');

expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');

router.urlUpdateStrategy = 'eager';

let urlAtNavStart = '';
let urlAtRoutesRecognized = '';
router.events.subscribe(e => {
if (e instanceof NavigationStart) {
urlAtNavStart = location.path();
}
if (e instanceof RoutesRecognized) {
urlAtRoutesRecognized = location.path();
}
});

router.navigateByUrl('/team/33');

advance(fixture);
expect(urlAtNavStart).toBe('/team/22');
expect(urlAtRoutesRecognized).toBe('/team/33');
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
})));

});

it('should navigate back and forward',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
@@ -4627,6 +4673,10 @@ class DummyLinkCmp {
}
}

@Component({selector: 'link-cmp', template: `<a [routerLink]="['/simple']">link</a>`})
class AbsoluteSimpleLinkCmp {
}

@Component({selector: 'link-cmp', template: `<a [routerLink]="['../simple']">link</a>`})
class RelativeLinkCmp {
}
@@ -4807,8 +4857,8 @@ class ThrowingCmp {



function advance(fixture: ComponentFixture<any>): void {
tick();
function advance(fixture: ComponentFixture<any>, millis?: number): void {
tick(millis);
fixture.detectChanges();
}

@@ -4836,6 +4886,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
@@ -4864,6 +4915,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
@@ -4894,6 +4946,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,

0 comments on commit ad26cd6

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.