From b8f80ab2dc873d348fc20fe3afd85aba98fe8dee Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 30 Apr 2025 10:05:05 -0700 Subject: [PATCH] refactor(router): Remove unnecessary runOutsideAngular in view transition helper (#61068) This refactor removes the unnecessary `runOutsideAngular` call in the view transition helper. The resolved promise re-enters the zone in the transition, so that will trigger the Angular zone anyways. If it didn't do that, it risks activated the route outside the zone, which is a bigger risk. Regardless, this function is only run once per navigation, so even if it _did_ result in an extra promise/timeout inside the zone, this is not excessive. Using ZoneJS to trigger rendering is known to overreact to events. Using `OnPush` or zoneless is more effective at mitigating this issue. PR Close #61068 --- packages/router/src/utils/view_transition.ts | 56 +++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/packages/router/src/utils/view_transition.ts b/packages/router/src/utils/view_transition.ts index 424fd240509..88ac2953e15 100644 --- a/packages/router/src/utils/view_transition.ts +++ b/packages/router/src/utils/view_transition.ts @@ -7,13 +7,7 @@ */ import {DOCUMENT} from '@angular/common'; -import { - afterNextRender, - InjectionToken, - Injector, - NgZone, - runInInjectionContext, -} from '@angular/core'; +import {afterNextRender, InjectionToken, Injector, runInInjectionContext} from '@angular/core'; import {ActivatedRouteSnapshot} from '../router_state'; @@ -83,33 +77,31 @@ export function createViewTransition( const transitionOptions = injector.get(VIEW_TRANSITION_OPTIONS); const document = injector.get(DOCUMENT); // Create promises outside the Angular zone to avoid causing extra change detections - return injector.get(NgZone).runOutsideAngular(() => { - if (!document.startViewTransition || transitionOptions.skipNextTransition) { - transitionOptions.skipNextTransition = false; - // The timing of `startViewTransition` is closer to a macrotask. It won't be called - // until the current event loop exits so we use a promise resolved in a timeout instead - // of Promise.resolve(). - return new Promise((resolve) => setTimeout(resolve)); - } + if (!document.startViewTransition || transitionOptions.skipNextTransition) { + transitionOptions.skipNextTransition = false; + // The timing of `startViewTransition` is closer to a macrotask. It won't be called + // until the current event loop exits so we use a promise resolved in a timeout instead + // of Promise.resolve(). + return new Promise((resolve) => setTimeout(resolve)); + } - let resolveViewTransitionStarted: () => void; - const viewTransitionStarted = new Promise((resolve) => { - resolveViewTransitionStarted = resolve; - }); - const transition = document.startViewTransition(() => { - resolveViewTransitionStarted(); - // We don't actually update dom within the transition callback. The resolving of the above - // promise unblocks the Router navigation, which synchronously activates and deactivates - // routes (the DOM update). This view transition waits for the next change detection to - // complete (below), which includes the update phase of the routed components. - return createRenderPromise(injector); - }); - const {onViewTransitionCreated} = transitionOptions; - if (onViewTransitionCreated) { - runInInjectionContext(injector, () => onViewTransitionCreated({transition, from, to})); - } - return viewTransitionStarted; + let resolveViewTransitionStarted: () => void; + const viewTransitionStarted = new Promise((resolve) => { + resolveViewTransitionStarted = resolve; }); + const transition = document.startViewTransition(() => { + resolveViewTransitionStarted(); + // We don't actually update dom within the transition callback. The resolving of the above + // promise unblocks the Router navigation, which synchronously activates and deactivates + // routes (the DOM update). This view transition waits for the next change detection to + // complete (below), which includes the update phase of the routed components. + return createRenderPromise(injector); + }); + const {onViewTransitionCreated} = transitionOptions; + if (onViewTransitionCreated) { + runInInjectionContext(injector, () => onViewTransitionCreated({transition, from, to})); + } + return viewTransitionStarted; } /**