From 5d7626b8f3b1349ee5e2a4d1c51db498f950400a Mon Sep 17 00:00:00 2001 From: Jon Snow Date: Tue, 3 Mar 2026 08:39:03 -0600 Subject: [PATCH 1/2] Fix ViewTransition onExit cleanup timing on unmount --- .../__tests__/ReactDOMViewTransition-test.js | 127 ++++++++++++++++++ .../src/ReactFiberCommitViewTransitions.js | 2 +- .../src/ReactFiberWorkLoop.js | 44 ++++-- 3 files changed, 161 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js b/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js index 1c5b43a18acd..2e07563bf97c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js @@ -18,6 +18,14 @@ let act; let assertLog; let Scheduler; let textCache; +let finishMockViewTransition; +let originalStartViewTransition; +let originalGetBoundingClientRect; +let originalCSS; +let originalGetAnimations; +let originalInnerWidth; +let originalInnerHeight; +let originalFonts; describe('ReactDOMViewTransition', () => { let container; @@ -38,9 +46,79 @@ describe('ReactDOMViewTransition', () => { document.body.appendChild(container); textCache = new Map(); + + finishMockViewTransition = null; + originalStartViewTransition = document.startViewTransition; + originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + originalCSS = global.CSS; + originalGetAnimations = document.documentElement.getAnimations; + originalInnerWidth = window.innerWidth; + originalInnerHeight = window.innerHeight; + originalFonts = document.fonts; + if (originalCSS == null || typeof originalCSS.escape !== 'function') { + global.CSS = { + escape: value => value, + }; + } + document.documentElement.getAnimations = function () { + return []; + }; + document.fonts = { + status: 'loaded', + ready: Promise.resolve(), + }; + Object.defineProperty(window, 'innerWidth', { + configurable: true, + value: 1024, + }); + Object.defineProperty(window, 'innerHeight', { + configurable: true, + value: 768, + }); + // Force visibility checks to pass in jsdom so transition callbacks run. + HTMLElement.prototype.getBoundingClientRect = function () { + return { + top: 1, + left: 1, + right: 11, + bottom: 11, + width: 10, + height: 10, + x: 1, + y: 1, + toJSON() {}, + }; + }; + document.startViewTransition = jest.fn(({update}) => { + const ready = Promise.resolve().then(() => update()); + return { + skipTransition() {}, + ready, + finished: new Promise(resolve => { + finishMockViewTransition = resolve; + }), + }; + }); }); afterEach(() => { + if (finishMockViewTransition !== null) { + finishMockViewTransition(); + finishMockViewTransition = null; + } + document.startViewTransition = originalStartViewTransition; + HTMLElement.prototype.getBoundingClientRect = originalGetBoundingClientRect; + global.CSS = originalCSS; + document.documentElement.getAnimations = originalGetAnimations; + document.fonts = originalFonts; + Object.defineProperty(window, 'innerWidth', { + configurable: true, + value: originalInnerWidth, + }); + Object.defineProperty(window, 'innerHeight', { + configurable: true, + value: originalInnerHeight, + }); document.body.removeChild(container); }); @@ -176,4 +254,53 @@ describe('ReactDOMViewTransition', () => { expect(container.textContent).toContain('Card 2'); expect(container.textContent).toContain('Card 3'); }); + + // @gate enableViewTransition && enableEffectEventMutationPhase + it('calls onExit cleanup immediately on unmount', async () => { + const log = []; + + function App({show}) { + return ( +
+ {show ? ( + { + log.push('exit'); + return () => { + log.push('cleanup'); + }; + }}> +
A
+ + ) : null} +
+ ); + } + + const root = ReactDOMClient.createRoot(container); + await act(async () => { + React.startTransition(() => { + root.render(); + }); + }); + document.startViewTransition.mockClear(); + + log.length = 0; + await act(async () => { + React.startTransition(() => { + root.render(); + }); + }); + expect(document.startViewTransition).toHaveBeenCalledTimes(1); + + expect(log).toEqual(['exit', 'cleanup']); + + if (finishMockViewTransition !== null) { + await act(() => { + finishMockViewTransition(); + }); + } + expect(log).toEqual(['exit', 'cleanup']); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index a9edc0c84d23..d2bc60138dfa 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -446,7 +446,7 @@ export function commitExitViewTransitions(deletion: Fiber): void { // Therefore it's possible for onShare to be called with only an old snapshot. scheduleViewTransitionEvent(deletion, props.onShare); } else { - scheduleViewTransitionEvent(deletion, props.onExit); + scheduleViewTransitionEvent(deletion, props.onExit, true); } } if (appearingViewTransitions !== null) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index d055b271ad77..78622c4537e5 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -735,9 +735,13 @@ let pendingEffectsRenderEndTime: number = -0; // Profiling-only let pendingPassiveTransitions: Array | null = null; let pendingRecoverableErrors: null | Array> = null; let pendingViewTransition: null | RunningViewTransition = null; -let pendingViewTransitionEvents: Array< - (types: Array) => void | (() => void), -> | null = null; +type PendingViewTransitionEvent = { + callback: (types: Array) => void | (() => void), + // If true, run the cleanup immediately after unmount. + cleanupOnUnmount: boolean, +}; +let pendingViewTransitionEvents: Array | null = + null; let pendingTransitionTypes: null | TransitionTypes = null; let pendingDidIncludeRenderPhaseUpdate: boolean = false; let pendingSuspendedCommitReason: SuspendedCommitReason = null; // Profiling-only @@ -906,6 +910,7 @@ export function scheduleViewTransitionEvent( instance: ViewTransitionInstance, types: Array, ) => void | (() => void), + cleanupOnUnmount?: boolean, ): void { if (enableViewTransition) { if (callback != null) { @@ -919,7 +924,10 @@ export function scheduleViewTransitionEvent( if (pendingViewTransitionEvents === null) { pendingViewTransitionEvents = []; } - pendingViewTransitionEvents.push(callback.bind(null, instance)); + pendingViewTransitionEvents.push({ + callback: callback.bind(null, instance), + cleanupOnUnmount: cleanupOnUnmount === true, + }); } } } @@ -952,9 +960,10 @@ export function scheduleGestureTransitionEvent( if (pendingViewTransitionEvents === null) { pendingViewTransitionEvents = []; } - pendingViewTransitionEvents.push( - callback.bind(null, timeline, options, instance), - ); + pendingViewTransitionEvents.push({ + callback: callback.bind(null, timeline, options, instance), + cleanupOnUnmount: false, + }); } } } @@ -4276,10 +4285,18 @@ function flushSpawnedWork(): void { } if (committedViewTransition !== null) { for (let i = 0; i < pendingEvents.length; i++) { - const viewTransitionEvent = pendingEvents[i]; + const {callback: viewTransitionEvent, cleanupOnUnmount} = + pendingEvents[i]; const cleanup = viewTransitionEvent(pendingTypes); if (cleanup !== undefined) { - addViewTransitionFinishedListener(committedViewTransition, cleanup); + if (cleanupOnUnmount) { + cleanup(); + } else { + addViewTransitionFinishedListener( + committedViewTransition, + cleanup, + ); + } } } } @@ -4554,10 +4571,15 @@ function flushGestureAnimations(): void { const runningTransition = appliedGesture.running; if (runningTransition !== null) { for (let i = 0; i < pendingEvents.length; i++) { - const viewTransitionEvent = pendingEvents[i]; + const {callback: viewTransitionEvent, cleanupOnUnmount} = + pendingEvents[i]; const cleanup = viewTransitionEvent(pendingTypes); if (cleanup !== undefined) { - addViewTransitionFinishedListener(runningTransition, cleanup); + if (cleanupOnUnmount) { + cleanup(); + } else { + addViewTransitionFinishedListener(runningTransition, cleanup); + } } } } From 81414378cb74608bda969a6d3ea7935c2c7ebc3e Mon Sep 17 00:00:00 2001 From: Jon Snow Date: Tue, 3 Mar 2026 10:30:58 -0600 Subject: [PATCH 2/2] Stabilize ViewTransition onExit cleanup regression test --- .../__tests__/ReactDOMViewTransition-test.js | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js b/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js index 2e07563bf97c..3ef2c1bf49fa 100644 --- a/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMViewTransition-test.js @@ -16,6 +16,7 @@ let SuspenseList; let ViewTransition; let act; let assertLog; +let assertConsoleWarnDev; let Scheduler; let textCache; let finishMockViewTransition; @@ -37,6 +38,7 @@ describe('ReactDOMViewTransition', () => { Scheduler = require('scheduler'); act = require('internal-test-utils').act; assertLog = require('internal-test-utils').assertLog; + assertConsoleWarnDev = require('internal-test-utils').assertConsoleWarnDev; Suspense = React.Suspense; ViewTransition = React.ViewTransition; if (gate(flags => flags.enableSuspenseList)) { @@ -255,27 +257,23 @@ describe('ReactDOMViewTransition', () => { expect(container.textContent).toContain('Card 3'); }); - // @gate enableViewTransition && enableEffectEventMutationPhase + // @gate enableViewTransition it('calls onExit cleanup immediately on unmount', async () => { const log = []; function App({show}) { - return ( -
- {show ? ( - { - log.push('exit'); - return () => { - log.push('cleanup'); - }; - }}> -
A
- - ) : null} -
- ); + return show ? ( + { + log.push('exit'); + return () => { + log.push('cleanup'); + }; + }}> +
A
+ + ) : null; } const root = ReactDOMClient.createRoot(container); @@ -292,15 +290,16 @@ describe('ReactDOMViewTransition', () => { root.render(); }); }); + if (__DEV__) { + assertConsoleWarnDev([ + 'A flushSync update cancelled a View Transition because it was called ' + + 'while the View Transition was still preparing. To preserve the synchronous ' + + "semantics, React had to skip the View Transition. If you can, try to avoid " + + "flushSync() in a scenario that's likely to interfere.", + ]); + } expect(document.startViewTransition).toHaveBeenCalledTimes(1); expect(log).toEqual(['exit', 'cleanup']); - - if (finishMockViewTransition !== null) { - await act(() => { - finishMockViewTransition(); - }); - } - expect(log).toEqual(['exit', 'cleanup']); }); });