Skip to content

Commit

Permalink
Revert "revert: fix(router): ensure URL is updated after second redir…
Browse files Browse the repository at this point in the history 
…ect with UrlUpdateStrategy="eager" (angular#27523)"

This reverts commit eea2b0f.
  • Loading branch information
@jasonaden
jasonaden committed Jan 22, 2019
1 parent f99082f commit 301c6dc
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 50 deletions.
13 changes: 10 additions & 3 deletions packages/router/src/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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;
}
}),

Expand Down
147 changes: 100 additions & 47 deletions packages/router/test/integration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactor
import {ComponentFixture, TestBed, fakeAsync, 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, 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';

Expand Down Expand Up @@ -571,64 +571,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) => {
Expand Down Expand Up @@ -4656,6 +4702,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 {
}
Expand Down Expand Up @@ -4836,8 +4886,8 @@ class ThrowingCmp {



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

Expand Down Expand Up @@ -4865,6 +4915,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
Expand Down Expand Up @@ -4893,6 +4944,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
Expand Down Expand Up @@ -4923,6 +4975,7 @@ class LazyComponent {
StringLinkCmp,
DummyLinkCmp,
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
Expand Down

0 comments on commit 301c6dc

Please sign in to comment.