refactor(router): compress synchronous end to router navigation to single operator

The end of the Router navigation is a block of synchronous logic that
can be compressed into a single operator rather than splitting it across
several, making it harder to step through. The only benefit from the
split is automatic unsubscribe/cancellation, which we can replicate
with an additional 'shouldContinue' check before proceeding.

(cherry picked from commit b7d21e2)

Author: atscott

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -385,7 +385,6 @@
       "_stripOrigin",
       "_wasLastNodeCreated",
       "abortSignalToObservable",
-      "activateRoutes",
       "activeConsumer",
       "addAfterRenderSequencesForView",
       "addEmptyPathsToChildrenIfNeeded",

packages/router/src/navigation_transition.ts

@@ -58,7 +58,7 @@ import {
   isRedirectingNavigationCancelingError,
   redirectingNavigationError,
 } from './navigation_canceling_error';
-import {activateRoutes} from './operators/activate_routes';
+import {ActivateRoutes} from './operators/activate_routes';
 import {checkGuards} from './operators/check_guards';
 import {recognize} from './operators/recognize';
 import {resolveData} from './operators/resolve_data';
@@ -725,6 +725,7 @@ export class NavigationTransitions {
 
           switchTap(() => this.afterPreactivation()),
 
+          // TODO(atscott): Move this into the last block below.
           switchMap(() => {
             const {currentSnapshot, targetSnapshot} = overallTransitionState;
             const viewTransitionStarted = this.createViewTransition?.(
@@ -740,35 +741,56 @@ export class NavigationTransitions {
               : of(overallTransitionState);
           }),
 
+          // Ensure that if some observable used to drive the transition doesn't
+          // complete, the navigation still finalizes This should never happen, but
+          // this is done as a safety measure to avoid surfacing this error (#49567).
+          take(1),
+
           map((t: NavigationTransition) => {
             const targetRouterState = createRouterState(
               router.routeReuseStrategy,
               t.targetSnapshot!,
               t.currentRouterState,
             );
-            this.currentTransition = overallTransitionState = {...t, targetRouterState};
+            this.currentTransition = overallTransitionState = t = {...t, targetRouterState};
             this.currentNavigation.update((nav) => {
               nav!.targetRouterState = targetRouterState;
               return nav;
             });
-            return overallTransitionState;
-          }),
 
-          tap(() => {
             this.events.next(new BeforeActivateRoutes());
-          }),
+            if (!shouldContinueNavigation()) {
+              return;
+            }
 
-          activateRoutes(
-            this.rootContexts,
-            router.routeReuseStrategy,
-            (evt: Event) => this.events.next(evt),
-            this.inputBindingEnabled,
-          ),
+            new ActivateRoutes(
+              router.routeReuseStrategy,
+              overallTransitionState.targetRouterState!,
+              overallTransitionState.currentRouterState,
+              (evt: Event) => this.events.next(evt),
+              this.inputBindingEnabled,
+            ).activate(this.rootContexts);
+
+            if (!shouldContinueNavigation()) {
+              return;
+            }
 
-          // Ensure that if some observable used to drive the transition doesn't
-          // complete, the navigation still finalizes This should never happen, but
-          // this is done as a safety measure to avoid surfacing this error (#49567).
-          take(1),
+            completedOrAborted = true;
+            this.currentNavigation.update((nav) => {
+              (nav as Writable<Navigation>).abort = noop;
+              return nav;
+            });
+            this.lastSuccessfulNavigation.set(untracked(this.currentNavigation));
+            this.events.next(
+              new NavigationEnd(
+                t.id,
+                this.urlSerializer.serialize(t.extractedUrl),
+                this.urlSerializer.serialize(t.urlAfterRedirects!),
+              ),
+            );
+            this.titleStrategy?.updateTitle(t.targetRouterState!.snapshot);
+            t.resolve(true);
+          }),
 
           takeUntil(
             abortSignalToObservable(abortController.signal).pipe(
@@ -785,23 +807,6 @@ export class NavigationTransitions {
           ),
 
           tap({
-            next: (t: NavigationTransition) => {
-              completedOrAborted = true;
-              this.currentNavigation.update((nav) => {
-                (nav as Writable<Navigation>).abort = noop;
-                return nav;
-              });
-              this.lastSuccessfulNavigation.set(untracked(this.currentNavigation));
-              this.events.next(
-                new NavigationEnd(
-                  t.id,
-                  this.urlSerializer.serialize(t.extractedUrl),
-                  this.urlSerializer.serialize(t.urlAfterRedirects!),
-                ),
-              );
-              this.titleStrategy?.updateTitle(t.targetRouterState!.snapshot);
-              t.resolve(true);
-            },
             complete: () => {
               completedOrAborted = true;
             },
@@ -849,6 +854,7 @@ export class NavigationTransitions {
             }
           }),
           catchError((e) => {
+            completedOrAborted = true;
             // If the application is already destroyed, the catch block should not
             // execute anything in practice because other resources have already
             // been released and destroyed.
@@ -857,7 +863,6 @@ export class NavigationTransitions {
               return EMPTY;
             }
 
-            completedOrAborted = true;
             /* This error type is issued during Redirect, and is handled as a
              * cancellation rather than an error. */
             if (isNavigationCancelingError(e)) {

packages/router/src/operators/activate_routes.ts

@@ -6,35 +6,13 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {MonoTypeOperatorFunction} from 'rxjs';
-import {map} from 'rxjs/operators';
-
 import {ActivationEnd, ChildActivationEnd, Event} from '../events';
-import type {NavigationTransition} from '../navigation_transition';
 import type {DetachedRouteHandleInternal, RouteReuseStrategy} from '../route_reuse_strategy';
 import type {ChildrenOutletContexts} from '../router_outlet_context';
 import {ActivatedRoute, advanceActivatedRoute, RouterState} from '../router_state';
 import {nodeChildrenAsMap, TreeNode} from '../utils/tree';
 
 let warnedAboutUnsupportedInputBinding = false;
-
-export const activateRoutes = (
-  rootContexts: ChildrenOutletContexts,
-  routeReuseStrategy: RouteReuseStrategy,
-  forwardEvent: (evt: Event) => void,
-  inputBindingEnabled: boolean,
-): MonoTypeOperatorFunction<NavigationTransition> =>
-  map((t) => {
-    new ActivateRoutes(
-      routeReuseStrategy,
-      t.targetRouterState!,
-      t.currentRouterState,
-      forwardEvent,
-      inputBindingEnabled,
-    ).activate(rootContexts);
-    return t;
-  });
-
 export class ActivateRoutes {
   constructor(
     private routeReuseStrategy: RouteReuseStrategy,