Skip to content
Permalink
Browse files

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

…teStrategy="eager" (angular#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 angular#27116

PR Close angular#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.