Skip to content

Commit eea2b0f

Browse files
matskomatsko
committed
revert: fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (angular#27523)
1 parent 13eb57a commit eea2b0f

2 files changed

Lines changed: 50 additions & 110 deletions

File tree

‎packages/router/src/router.ts‎

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

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

506504
// Update URL if in `eager` update mode
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-
}),
505+
tap(t => this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange &&
506+
this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id)),
513507

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

672666
if (this.urlUpdateStrategy === 'deferred' && !t.extras.skipLocationChange) {
673667
this.setBrowserUrl(this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
674-
this.browserUrlTree = t.urlAfterRedirects;
675668
}
676669
}),
677670

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

Lines changed: 47 additions & 100 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, 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';
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';
1717
import {Observable, Observer, Subscription, of } from 'rxjs';
1818
import {filter, first, map, tap} from 'rxjs/operators';
1919

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

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-
});
589-
590-
});
591-
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);
592582

593-
it('should eagerly update the URL with urlUpdateStrategy="eagar"',
594-
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
595-
const fixture = TestBed.createComponent(RootCmp);
596-
advance(fixture);
583+
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
597584

598-
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
585+
router.navigateByUrl('/team/22');
586+
advance(fixture);
587+
expect(location.path()).toEqual('/team/22');
599588

600-
router.navigateByUrl('/team/22');
601-
advance(fixture);
602-
expect(location.path()).toEqual('/team/22');
589+
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
603590

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

606-
router.urlUpdateStrategy = 'eager';
607-
(router as any).hooks.beforePreactivation = () => {
608-
expect(location.path()).toEqual('/team/33');
609-
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
610-
return of (null);
611-
};
612-
router.navigateByUrl('/team/33');
613-
614-
advance(fixture);
615-
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
616-
})));
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);
621-
622-
router.urlUpdateStrategy = 'eager';
623-
624-
router.resetConfig([
625-
{path: 'team/:id', component: SimpleCmp, canActivate: ['authGuardFail']},
626-
{path: 'login', component: AbsoluteSimpleLinkCmp}
627-
]);
628-
629-
router.navigateByUrl('/team/22');
630-
advance(fixture);
631-
expect(location.path()).toEqual('/team/22');
632-
633-
// Redirects to /login
634-
advance(fixture, 1);
635-
expect(location.path()).toEqual('/login');
636-
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');
641-
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);
599+
advance(fixture);
600+
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
601+
})));
651602

652-
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
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);
653607

654-
router.navigateByUrl('/team/22');
655-
advance(fixture);
656-
expect(location.path()).toEqual('/team/22');
608+
router.resetConfig([{path: 'team/:id', component: TeamCmp}]);
657609

658-
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
610+
router.navigateByUrl('/team/22');
611+
advance(fixture);
612+
expect(location.path()).toEqual('/team/22');
659613

660-
router.urlUpdateStrategy = 'eager';
614+
expect(fixture.nativeElement).toHaveText('team 22 [ , right: ]');
661615

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-
});
616+
router.urlUpdateStrategy = 'eager';
672617

673-
router.navigateByUrl('/team/33');
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+
});
674628

675-
advance(fixture);
676-
expect(urlAtNavStart).toBe('/team/22');
677-
expect(urlAtRoutesRecognized).toBe('/team/33');
678-
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
679-
})));
629+
router.navigateByUrl('/team/33');
680630

681-
});
631+
advance(fixture);
632+
expect(urlAtNavStart).toBe('/team/22');
633+
expect(urlAtRoutesRecognized).toBe('/team/33');
634+
expect(fixture.nativeElement).toHaveText('team 33 [ , right: ]');
635+
})));
682636

683637
it('should navigate back and forward',
684638
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
@@ -4713,10 +4667,6 @@ class DummyLinkCmp {
47134667
}
47144668
}
47154669

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

48984848

48994849

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

@@ -4926,7 +4876,6 @@ class LazyComponent {
49264876
StringLinkCmp,
49274877
DummyLinkCmp,
49284878
AbsoluteLinkCmp,
4929-
AbsoluteSimpleLinkCmp,
49304879
RelativeLinkCmp,
49314880
DummyLinkWithParentCmp,
49324881
LinkWithQueryParamsAndFragment,
@@ -4955,7 +4904,6 @@ class LazyComponent {
49554904
StringLinkCmp,
49564905
DummyLinkCmp,
49574906
AbsoluteLinkCmp,
4958-
AbsoluteSimpleLinkCmp,
49594907
RelativeLinkCmp,
49604908
DummyLinkWithParentCmp,
49614909
LinkWithQueryParamsAndFragment,
@@ -4986,7 +4934,6 @@ class LazyComponent {
49864934
StringLinkCmp,
49874935
DummyLinkCmp,
49884936
AbsoluteLinkCmp,
4989-
AbsoluteSimpleLinkCmp,
49904937
RelativeLinkCmp,
49914938
DummyLinkWithParentCmp,
49924939
LinkWithQueryParamsAndFragment,

0 commit comments

Comments
 (0)