fix(router): Navigations triggered by cancellation events should canc…

…el previous navigation (#54710)

There is an edge case where synchronous navigations caused in
response to navigation events can result in a previous navigation not
being unsubscribed from. b/328219996

PR Close #54710

packages/router/src/navigation_transition.ts

@@ -430,12 +430,28 @@ export class NavigationTransitions {
 
       // Using switchMap so we cancel executing navigations when a new one comes in
       switchMap((overallTransitionState) => {
-        this.currentTransition = overallTransitionState;
         let completed = false;
         let errored = false;
         return of(overallTransitionState).pipe(
-          // Store the Navigation object
-          tap((t) => {
+          switchMap((t) => {
+            // It is possible that `switchMap` fails to cancel previous navigations if a new one happens synchronously while the operator
+            // is processing the `next` notification of that previous navigation. This can happen when a new navigation (say 2) cancels a
+            // previous one (1) and yet another navigation (3) happens synchronously in response to the `NavigationCancel` event for (1).
+            // https://github.com/ReactiveX/rxjs/issues/7455
+            if (this.navigationId > overallTransitionState.id) {
+              const cancellationReason =
+                typeof ngDevMode === 'undefined' || ngDevMode
+                  ? `Navigation ID ${overallTransitionState.id} is not equal to the current navigation id ${this.navigationId}`
+                  : '';
+              this.cancelNavigationTransition(
+                overallTransitionState,
+                cancellationReason,
+                NavigationCancellationCode.SupersededByNewNavigation,
+              );
+              return EMPTY;
+            }
+            this.currentTransition = overallTransitionState;
+            // Store the Navigation object
             this.currentNavigation = {
               id: t.id,
               initialUrl: t.rawUrl,
@@ -449,8 +465,6 @@ export class NavigationTransitions {
                     previousNavigation: null,
                   },
             };
-          }),
-          switchMap((t) => {
             const urlTransition =
               !router.navigated || this.isUpdatingInternalState() || this.isUpdatedBrowserUrl();
 

packages/router/test/regression_integration.spec.ts

@@ -23,16 +23,19 @@ import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'
 import {
   ChildrenOutletContexts,
   DefaultUrlSerializer,
+  NavigationCancel,
+  NavigationError,
   Router,
   RouterModule,
   RouterOutlet,
   UrlSerializer,
   UrlTree,
 } from '@angular/router';
 import {of} from 'rxjs';
-import {delay, mapTo} from 'rxjs/operators';
+import {delay, filter, mapTo, take} from 'rxjs/operators';
 
-import {provideRouter} from '../src/provide_router';
+import {provideRouter, withRouterConfig} from '../src/provide_router';
+import {afterNextNavigation} from '../src/utils/navigations';
 
 describe('Integration', () => {
   describe('routerLinkActive', () => {
@@ -418,6 +421,48 @@ describe('Integration', () => {
 
     expect(router.url).toEqual(`/?q=${SPECIAL_SERIALIZATION}`);
   });
+
+  it('navigation works when a redirecting NavigationCancel event causes another synchronous navigation', async () => {
+    TestBed.configureTestingModule({
+      providers: [
+        provideRouter(
+          [
+            {path: 'a', children: []},
+            {path: 'b', children: []},
+            {path: 'c', children: []},
+          ],
+          withRouterConfig({resolveNavigationPromiseOnError: true}),
+        ),
+      ],
+    });
+
+    let errors: NavigationError[] = [];
+    let cancellations: NavigationCancel[] = [];
+    const router = TestBed.inject(Router);
+    router.events
+      .pipe(filter((e): e is NavigationError => e instanceof NavigationError))
+      .subscribe((e) => errors.push(e));
+    router.events
+      .pipe(filter((e): e is NavigationCancel => e instanceof NavigationCancel))
+      .subscribe((e) => cancellations.push(e));
+
+    router.events
+      .pipe(
+        filter((e) => e instanceof NavigationCancel),
+        take(1),
+      )
+      .subscribe(() => {
+        router.navigateByUrl('/c');
+      });
+    router.navigateByUrl('/a');
+    router.navigateByUrl('/b');
+    await new Promise<void>((resolve) => afterNextNavigation(router, resolve));
+
+    expect(router.url).toEqual('/c');
+    expect(errors).toEqual([]);
+    // navigations to a and b were both cancelled.
+    expect(cancellations.length).toEqual(2);
+  });
 });
 
 function advance<T>(fixture: ComponentFixture<T>): void {