revert: fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523)

packages/router/src/router.ts

@@ -291,7 +291,6 @@ 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;
@@ -401,7 +400,6 @@ 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);
@@ -463,7 +461,7 @@ export class Router {
           return of (t).pipe(
               switchMap(t => {
                 const urlTransition =
-                    !this.navigated || t.extractedUrl.toString() !== this.browserUrlTree.toString();
+                    !this.navigated || t.extractedUrl.toString() !== this.currentUrlTree.toString();
                 const processCurrentUrl =
                     (this.onSameUrlNavigation === 'reload' ? true : urlTransition) &&
                     this.urlHandlingStrategy.shouldProcessUrl(t.rawUrl);
@@ -504,12 +502,8 @@ export class Router {
                           this.paramsInheritanceStrategy, this.relativeLinkResolution),
 
                       // Update URL if in `eager` update mode
-                      tap(t => {
+                      tap(t => this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange &&
-                        if (this.urlUpdateStrategy === 'eager' && !t.extras.skipLocationChange) {
+                              this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id)),
-                          this.setBrowserUrl(t.urlAfterRedirects, !!t.extras.replaceUrl, t.id);
-                          this.browserUrlTree = t.urlAfterRedirects;
-                        }
-                      }),
 
                       // Fire RoutesRecognized
                       tap(t => {
@@ -671,7 +665,6 @@ 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;
                 }
               }),
 

packages/router/test/integration.spec.ts

@@ -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, 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 {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 {Observable, Observer, Subscription, of } from 'rxjs';
 import {filter, first, map, tap} from 'rxjs/operators';
 
@@ -575,21 +575,6 @@ describe('Integration', () => {
        expect(fixture.nativeElement).toHaveText('team 33 [ , right:  ]');
      })));
 
-  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); });
-          }
-        }]
-      });
-
-    });
-
-
   it('should eagerly update the URL with urlUpdateStrategy="eagar"',
      fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
        const fixture = TestBed.createComponent(RootCmp);
@@ -614,35 +599,6 @@ describe('Integration', () => {
        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.urlUpdateStrategy = 'eager';
-
-         router.resetConfig([
-           {path: 'team/:id', component: SimpleCmp, canActivate: ['authGuardFail']},
-           {path: 'login', component: AbsoluteSimpleLinkCmp}
-         ]);
-
-         router.navigateByUrl('/team/22');
-         advance(fixture);
-         expect(location.path()).toEqual('/team/22');
-
-         // Redirects to /login
-         advance(fixture, 1);
-         expect(location.path()).toEqual('/login');
-
-         // Perform the same logic again, and it should produce the same result
-         router.navigateByUrl('/team/22');
-         advance(fixture);
-         expect(location.path()).toEqual('/team/22');
-
-         // Redirects to /login
-         advance(fixture, 1);
-         expect(location.path()).toEqual('/login');
-       })));
 
   it('should eagerly update URL after redirects are applied with urlUpdateStrategy="eagar"',
      fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
@@ -678,8 +634,6 @@ describe('Integration', () => {
        expect(fixture.nativeElement).toHaveText('team 33 [ , right:  ]');
      })));
 
-  });
-
   it('should navigate back and forward',
      fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
        const fixture = createRoot(router, RootCmp);
@@ -4713,10 +4667,6 @@ 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 {
 }
@@ -4897,8 +4847,8 @@ class ThrowingCmp {
 
 
 
-function advance(fixture: ComponentFixture<any>, millis?: number): void {
+function advance(fixture: ComponentFixture<any>): void {
-  tick(millis);
+  tick();
   fixture.detectChanges();
 }
 
@@ -4926,7 +4876,6 @@ class LazyComponent {
     StringLinkCmp,
     DummyLinkCmp,
     AbsoluteLinkCmp,
-    AbsoluteSimpleLinkCmp,
     RelativeLinkCmp,
     DummyLinkWithParentCmp,
     LinkWithQueryParamsAndFragment,
@@ -4955,7 +4904,6 @@ class LazyComponent {
     StringLinkCmp,
     DummyLinkCmp,
     AbsoluteLinkCmp,
-    AbsoluteSimpleLinkCmp,
     RelativeLinkCmp,
     DummyLinkWithParentCmp,
     LinkWithQueryParamsAndFragment,
@@ -4986,7 +4934,6 @@ class LazyComponent {
     StringLinkCmp,
     DummyLinkCmp,
     AbsoluteLinkCmp,
-    AbsoluteSimpleLinkCmp,
     RelativeLinkCmp,
     DummyLinkWithParentCmp,
     LinkWithQueryParamsAndFragment,