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.
This commit is contained in:
Andrew Scott 2025-10-14 17:10:46 -07:00 committed by Pawel Kozlowski
parent e30e61b789
commit b7d21e209a
3 changed files with 40 additions and 58 deletions

View file

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

View file

@ -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,36 +741,57 @@ 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;
}
new ActivateRoutes(
router.routeReuseStrategy,
overallTransitionState.targetRouterState!,
overallTransitionState.currentRouterState,
(evt: Event) => this.events.next(evt),
this.inputBindingEnabled,
).activate(this.rootContexts);
if (!shouldContinueNavigation()) {
return;
}
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);
}),
activateRoutes(
this.rootContexts,
router.routeReuseStrategy,
(evt: Event) => this.events.next(evt),
this.inputBindingEnabled,
),
// 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),
takeUntil(
abortSignalToObservable(abortController.signal).pipe(
// Ignore aborts if we are already completed, canceled, or are in the activation stage (we have targetRouterState)
@ -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)) {

View file

@ -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,