Skip to content

Commit 6ae7aee

Browse files
jasonadenAndrewKushnir
jasonaden
authored and
AndrewKushnir
committed
fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27680)
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 #27680
1 parent 701270d commit 6ae7aee

2 files changed

Lines changed: 110 additions & 50 deletions

File tree

‎packages/router/src/router.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ function defaultRouterHook(snapshot: RouterStateSnapshot, runExtras: {
291291
export class Router {
292292
private currentUrlTree: UrlTree;
293293
private rawUrlTree: UrlTree;
294+
private browserUrlTree: UrlTree;
294295
private readonly transitions: BehaviorSubject<NavigationTransition>;
295296
private navigations: Observable<NavigationTransition>;
296297
private lastSuccessfulNavigation: Navigation|null = null;
@@ -400,6 +401,7 @@ export class Router {
400401
this.resetConfig(config);
401402
this.currentUrlTree = createEmptyUrlTree();
402403
this.rawUrlTree = this.currentUrlTree;
404+
this.browserUrlTree = this.parseUrl(this.location.path());
403405

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

504506
// Update URL if in `eager` update mode
505-
tap(t => this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange &&
506-
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id)),
507+
tap(t => {
508+
if (this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange) {
509+
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id);
510+
this.browserUrlTree = t.urlAfterRedirects;
511+
}
512+
}),
507513

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

666672
if (this.urlUpdateStrategy === 'deferred' && !t.extras.skipLocationChange) {
667673
this.setBrowserUrl(this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
674+
this.browserUrlTree = t.urlAfterRedirects;
668675
}
669676
}),
670677

‎packages/router/test/integration.spec.ts‎

Lines changed: 100 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/
1313
import {By} from '@angular/platform-browser/src/dom/debug/by';
1414
import {expect} from '@angular/platform-browser/testing/src/matchers';
1515
import {fixmeIvy} from '@angular/private/testing';
16-
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';
16+
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';
1717
import {Observable, Observer, Subscription, of } from 'rxjs';
1818
import {filter, first, map, tap} from 'rxjs/operators';
1919

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

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

583-
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
590+
});
584591

585-
router.navigateByUrl('/team/22');
586-
advance(fixture);
587-
expect(location.path()).toEqual('/team/22');
592+
it('should eagerly update the URL with urlUpdateStrategy="eagar"',
593+
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
594+
const fixture = TestBed.createComponent(RootCmp);
595+
advance(fixture);
588596

589-
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
597+
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
598+
599+
router.navigateByUrl('/team/22');
600+
advance(fixture);
601+
expect(location.path()).toEqual('/team/22');
590602

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

599-
advance(fixture);
600-
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
601-
})));
605+
router.urlUpdateStrategy = 'eager';
606+
(router as any).hooks.beforePreactivation = () => {
607+
expect(location.path()).toEqual('/team/33');
608+
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
609+
return of (null);
610+
};
611+
router.navigateByUrl('/team/33');
602612

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

608-
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
617+
it('should eagerly update the URL with urlUpdateStrategy="eagar"',
618+
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
619+
const fixture = TestBed.createComponent(RootCmp);
620+
advance(fixture);
609621

610-
router.navigateByUrl('/team/22');
611-
advance(fixture);
612-
expect(location.path()).toEqual('/team/22');
622+
router.urlUpdateStrategy = 'eager';
613623

614-
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
624+
router.resetConfig([
625+
{path: 'team/:id', component: SimpleCmp, canActivate: ['authGuardFail']},
626+
{path: 'login', component: AbsoluteSimpleLinkCmp}
627+
]);
615628

616-
router.urlUpdateStrategy = 'eager';
629+
router.navigateByUrl('/team/22');
630+
advance(fixture);
631+
expect(location.path()).toEqual('/team/22');
617632

618-
let urlAtNavStart = '';
619-
let urlAtRoutesRecognized = '';
620-
router.events.subscribe(e => {
621-
if (e instanceof NavigationStart) {
622-
urlAtNavStart = location.path();
623-
}
624-
if (e instanceof RoutesRecognized) {
625-
urlAtRoutesRecognized = location.path();
626-
}
627-
});
633+
// Redirects to /login
634+
advance(fixture, 1);
635+
expect(location.path()).toEqual('/login');
628636

629-
router.navigateByUrl('/team/33');
637+
// Perform the same logic again, and it should produce the same result
638+
router.navigateByUrl('/team/22');
639+
advance(fixture);
640+
expect(location.path()).toEqual('/team/22');
630641

631-
advance(fixture);
632-
expect(urlAtNavStart).toBe('/team/22');
633-
expect(urlAtRoutesRecognized).toBe('/team/33');
634-
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
635-
})));
642+
// Redirects to /login
643+
advance(fixture, 1);
644+
expect(location.path()).toEqual('/login');
645+
})));
646+
647+
it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eagar"',
648+
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
649+
const fixture = TestBed.createComponent(RootCmp);
650+
advance(fixture);
651+
652+
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
653+
654+
router.navigateByUrl('/team/22');
655+
advance(fixture);
656+
expect(location.path()).toEqual('/team/22');
657+
658+
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
659+
660+
router.urlUpdateStrategy = 'eager';
661+
662+
let urlAtNavStart = '';
663+
let urlAtRoutesRecognized = '';
664+
router.events.subscribe(e => {
665+
if (e instanceof NavigationStart) {
666+
urlAtNavStart = location.path();
667+
}
668+
if (e instanceof RoutesRecognized) {
669+
urlAtRoutesRecognized = location.path();
670+
}
671+
});
672+
673+
router.navigateByUrl('/team/33');
674+
675+
advance(fixture);
676+
expect(urlAtNavStart).toBe('/team/22');
677+
expect(urlAtRoutesRecognized).toBe('/team/33');
678+
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
679+
})));
680+
681+
});
636682

637683
it('should navigate back and forward',
638684
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
@@ -4667,6 +4713,10 @@ class DummyLinkCmp {
46674713
}
46684714
}
46694715

4716+
@Component({selector: 'link-cmp', template: `<a [routerLink]="['/simple']">link</a>`})
4717+
class AbsoluteSimpleLinkCmp {
4718+
}
4719+
46704720
@Component({selector: 'link-cmp', template: `<a [routerLink]="['../simple']">link</a>`})
46714721
class RelativeLinkCmp {
46724722
}
@@ -4847,8 +4897,8 @@ class ThrowingCmp {
48474897

48484898

48494899

4850-
function advance(fixture: ComponentFixture<any>): void {
4851-
tick();
4900+
function advance(fixture: ComponentFixture<any>, millis?: number): void {
4901+
tick(millis);
48524902
fixture.detectChanges();
48534903
}
48544904

@@ -4876,6 +4926,7 @@ class LazyComponent {
48764926
StringLinkCmp,
48774927
DummyLinkCmp,
48784928
AbsoluteLinkCmp,
4929+
AbsoluteSimpleLinkCmp,
48794930
RelativeLinkCmp,
48804931
DummyLinkWithParentCmp,
48814932
LinkWithQueryParamsAndFragment,
@@ -4904,6 +4955,7 @@ class LazyComponent {
49044955
StringLinkCmp,
49054956
DummyLinkCmp,
49064957
AbsoluteLinkCmp,
4958+
AbsoluteSimpleLinkCmp,
49074959
RelativeLinkCmp,
49084960
DummyLinkWithParentCmp,
49094961
LinkWithQueryParamsAndFragment,
@@ -4934,6 +4986,7 @@ class LazyComponent {
49344986
StringLinkCmp,
49354987
DummyLinkCmp,
49364988
AbsoluteLinkCmp,
4989+
AbsoluteSimpleLinkCmp,
49374990
RelativeLinkCmp,
49384991
DummyLinkWithParentCmp,
49394992
LinkWithQueryParamsAndFragment,

0 commit comments

Comments
 (0)