From 800c9db22e69680f17e238724478537282215f89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 8 Jan 2025 18:57:54 -0500 Subject: [PATCH 1/2] ViewTransitions in Navigation (#32028) This adds navigation support to the View Transition fixture using both `history.pushState/popstate` and the Navigation API models. Because `popstate` does scroll restoration synchronously at the end of the event, but `startViewTransition` cannot start synchronously, it would observe the "old" state as after applying scroll restoration. This leads to weird artifacts. So we intentionally do not support View Transitions in `popstate`. If it suspends anyway for some other reason, then scroll restoration is broken anyway and then it is supported. We don't have to do anything here because this is already how things worked because the sync `popstate` special case already included the sync lane which opts it out of View Transitions. For the Navigation API, scroll restoration can be blocked. The best way to do this is to resolve the Navigation API promise after React has applied its mutation. We can detect if there's currently any pending navigation and wait to resolve the `startViewTransition` until it finishes and any scroll restoration has been applied. https://github.com/user-attachments/assets/f53b3282-6315-4513-b3d6-b8981d66964e There is a subtle thing here. If we read the viewport metrics before scroll restoration has been applied, then we might assume something is or isn't going to be within the viewport incorrectly. This is evident on the "Slide In from Left" example. When we're going forward to that page we shift the scroll position such that it's going to appear in the viewport. If we did this before applying scroll restoration, it would not animate because it wasn't in the viewport then. Therefore, we need to run the after mutation phase after scroll restoration. A consequence of this is that you have to resolve Navigation in `useInsertionEffect` as otherwise it leads to a deadlock (which eventually gets broken by `startViewTransition`'s timeout of 10 seconds). Another consequence is that now `useLayoutEffect` observes the restored state. However, I think what we'll likely do is move the layout phase to before the after mutation phase which also ensures that auto-scrolling inside `useLayoutEffect` are considered in the viewport measurements as well. --- fixtures/view-transition/server/render.js | 39 +++++----- .../view-transition/src/components/App.js | 73 ++++++++++++++++++- .../view-transition/src/components/Page.js | 25 +++---- fixtures/view-transition/src/index.js | 8 +- .../src/client/ReactFiberConfigDOM.js | 32 +++++++- .../react-reconciler/src/ReactFiberLane.js | 4 + .../src/ReactFiberWorkLoop.js | 8 +- 7 files changed, 149 insertions(+), 40 deletions(-) diff --git a/fixtures/view-transition/server/render.js b/fixtures/view-transition/server/render.js index 0d956fd66caf7..11d352eabdd72 100644 --- a/fixtures/view-transition/server/render.js +++ b/fixtures/view-transition/server/render.js @@ -20,24 +20,27 @@ export default function render(url, res) { console.error('Fatal', error); }); let didError = false; - const {pipe, abort} = renderToPipeableStream(, { - bootstrapScripts: [assets['main.js']], - onShellReady() { - // If something errored before we started streaming, we set the error code appropriately. - res.statusCode = didError ? 500 : 200; - res.setHeader('Content-type', 'text/html'); - pipe(res); - }, - onShellError(x) { - // Something errored before we could complete the shell so we emit an alternative shell. - res.statusCode = 500; - res.send('

Error

'); - }, - onError(x) { - didError = true; - console.error(x); - }, - }); + const {pipe, abort} = renderToPipeableStream( + , + { + bootstrapScripts: [assets['main.js']], + onShellReady() { + // If something errored before we started streaming, we set the error code appropriately. + res.statusCode = didError ? 500 : 200; + res.setHeader('Content-type', 'text/html'); + pipe(res); + }, + onShellError(x) { + // Something errored before we could complete the shell so we emit an alternative shell. + res.statusCode = 500; + res.send('

Error

'); + }, + onError(x) { + didError = true; + console.error(x); + }, + } + ); // Abandon and switch to client rendering after 5 seconds. // Try lowering this to see the client recover. setTimeout(abort, 5000); diff --git a/fixtures/view-transition/src/components/App.js b/fixtures/view-transition/src/components/App.js index 6867b29d4c5a5..5fe8555cf8e64 100644 --- a/fixtures/view-transition/src/components/App.js +++ b/fixtures/view-transition/src/components/App.js @@ -1,12 +1,79 @@ -import React from 'react'; +import React, { + startTransition, + useInsertionEffect, + useEffect, + useState, +} from 'react'; import Chrome from './Chrome'; import Page from './Page'; -export default function App({assets}) { +const enableNavigationAPI = typeof navigation === 'object'; + +export default function App({assets, initialURL}) { + const [routerState, setRouterState] = useState({ + pendingNav: () => {}, + url: initialURL, + }); + function navigate(url) { + if (enableNavigationAPI) { + window.navigation.navigate(url); + } else { + startTransition(() => { + setRouterState({ + url, + pendingNav() { + window.history.pushState({}, '', url); + }, + }); + }); + } + } + useEffect(() => { + if (enableNavigationAPI) { + window.navigation.addEventListener('navigate', event => { + if (!event.canIntercept) { + return; + } + const newURL = new URL(event.destination.url); + event.intercept({ + handler() { + let promise; + startTransition(() => { + promise = new Promise(resolve => { + setRouterState({ + url: newURL.pathname + newURL.search, + pendingNav: resolve, + }); + }); + }); + return promise; + }, + commit: 'after-transition', // plz ship this, browsers + }); + }); + } else { + window.addEventListener('popstate', () => { + // This should not animate because restoration has to be synchronous. + // Even though it's a transition. + startTransition(() => { + setRouterState({ + url: document.location.pathname + document.location.search, + pendingNav() { + // Noop. URL has already updated. + }, + }); + }); + }); + } + }, []); + const pendingNav = routerState.pendingNav; + useInsertionEffect(() => { + pendingNav(); + }, [pendingNav]); return ( - + ); } diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 0ebc83bc91bac..ab40790b1647b 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -1,8 +1,5 @@ import React, { unstable_ViewTransition as ViewTransition, - startTransition, - useEffect, - useState, unstable_Activity as Activity, } from 'react'; @@ -37,13 +34,8 @@ function Component() { ); } -export default function Page() { - const [show, setShow] = useState(false); - useEffect(() => { - startTransition(() => { - setShow(true); - }); - }, []); +export default function Page({url, navigate}) { + const show = url === '/?b'; const exclamation = ( ! @@ -53,9 +45,7 @@ export default function Page() {
@@ -75,6 +65,15 @@ export default function Page() { {show ?
hello{exclamation}
:
Loading
} +

scroll me

+

+

+

+

+

+

+

+

{show ? null : (
world{exclamation}
diff --git a/fixtures/view-transition/src/index.js b/fixtures/view-transition/src/index.js index f6457ce570674..8c2fac3e67ada 100644 --- a/fixtures/view-transition/src/index.js +++ b/fixtures/view-transition/src/index.js @@ -3,4 +3,10 @@ import {hydrateRoot} from 'react-dom/client'; import App from './components/App'; -hydrateRoot(document, ); +hydrateRoot( + document, + +); diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 93b80f13479f2..aade021fdd039 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -1205,9 +1205,9 @@ export function startViewTransition( layoutCallback: () => void, passiveCallback: () => mixed, ): boolean { - const ownerDocument = + const ownerDocument: Document = rootContainer.nodeType === DOCUMENT_NODE - ? rootContainer + ? (rootContainer: any) : rootContainer.ownerDocument; try { // $FlowFixMe[prop-missing] @@ -1215,12 +1215,38 @@ export function startViewTransition( update() { mutationCallback(); // TODO: Wait for fonts. - afterMutationCallback(); + const ownerWindow = ownerDocument.defaultView; + const pendingNavigation = + ownerWindow.navigation && ownerWindow.navigation.transition; + if (pendingNavigation) { + return pendingNavigation.finished.then( + afterMutationCallback, + afterMutationCallback, + ); + } else { + afterMutationCallback(); + } }, types: null, // TODO: Provide types. }); // $FlowFixMe[prop-missing] ownerDocument.__reactViewTransition = transition; + if (__DEV__) { + transition.ready.then(undefined, (reason: mixed) => { + if ( + typeof reason === 'object' && + reason !== null && + reason.name === 'TimeoutError' + ) { + console.error( + 'A ViewTransition timed out because a Navigation stalled. ' + + 'This can happen if a Navigation is blocked on React itself. ' + + "Such as if it's resolved inside useLayoutEffect. " + + 'This can be solved by moving the resolution to useInsertionEffect.', + ); + } + }); + } transition.ready.then(layoutCallback, layoutCallback); transition.finished.then(() => { // $FlowFixMe[prop-missing] diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 11037d5f24053..922fe2f977d45 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -350,6 +350,10 @@ export function getNextLanesToFlushSync( // // The main use case is updates scheduled by popstate events, which are // flushed synchronously even though they are transitions. + // Note that we intentionally treat this as a sync flush to include any + // sync updates in a single pass but also intentionally disables View Transitions + // inside popstate. Because they can start synchronously before scroll restoration + // happens. const lanesToFlush = SyncUpdateLanes | extraLanesToForceSync; // Early bailout if there's no pending work left. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index d73e4f7002bd2..7a387b690b103 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3507,7 +3507,12 @@ function flushMutationEffects(): void { } function flushLayoutEffects(): void { - if (pendingEffectsStatus !== PENDING_LAYOUT_PHASE) { + if ( + pendingEffectsStatus !== PENDING_LAYOUT_PHASE && + // If a startViewTransition times out, we might flush this earlier than + // after mutation phase. In that case, we just skip the after mutation phase. + pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE + ) { return; } pendingEffectsStatus = NO_PENDING_EFFECTS; @@ -3790,7 +3795,6 @@ export function flushPendingEffects(wasDelayedCommit?: boolean): boolean { // Returns whether passive effects were flushed. flushMutationEffects(); flushLayoutEffects(); - flushAfterMutationEffects(); return flushPassiveEffects(wasDelayedCommit); } From fd9cfa416f7c01ecdf76b10ab776a43f2430786d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 8 Jan 2025 19:13:25 -0500 Subject: [PATCH 2/2] Execute layout phase before after mutation phase inside view transition (#32029) This allows mutations and scrolling in the layout phase to be counted towards the mutation. This would maybe not be the case for gestures but it is useful for fire-and-forget. This also avoids the issue that if you resolve navigation in useLayoutEffect that it ends up dead locked. It also means that useLayoutEffect does not observe the scroll restoration and in fact, the scroll restoration would win over any manual scrolling in layout effects. For better or worse, this is more in line with how things worked before and how it works in popstate. So it's less of a breaking change. This does mean that we can't unify the after mutation phase with the layout phase though. To do this we need split out flushSpawnedWork from the flushLayoutEffect call. Spawned work from setState inside the layout phase is done outside and not counted towards the transition. They're sync updates and so are not eligible for their own View Transitions. It's also tricky to support this since it's unclear what things like exits in that update would mean. This work will still be able to mutate the live DOM but it's just not eligible to trigger new transitions or adjust the target of those. One difference between popstate is that this spawned work is after scroll restoration. So any scrolling spawned from a second pass would now win over scroll restoration. Another consequence of this change is that you can't safely animate pseudo elements in useLayoutEffect. We'll introduce a better event for that anyway. --- .../view-transition/src/components/App.js | 4 +- .../src/client/ReactFiberConfigDOM.js | 17 ++++-- .../src/ReactFiberConfigNative.js | 3 +- .../src/ReactFiberWorkLoop.js | 57 ++++++++++++------- .../src/ReactFiberConfigTestHost.js | 3 +- 5 files changed, 52 insertions(+), 32 deletions(-) diff --git a/fixtures/view-transition/src/components/App.js b/fixtures/view-transition/src/components/App.js index 5fe8555cf8e64..028f511107c8b 100644 --- a/fixtures/view-transition/src/components/App.js +++ b/fixtures/view-transition/src/components/App.js @@ -1,6 +1,6 @@ import React, { startTransition, - useInsertionEffect, + useLayoutEffect, useEffect, useState, } from 'react'; @@ -68,7 +68,7 @@ export default function App({assets, initialURL}) { } }, []); const pendingNav = routerState.pendingNav; - useInsertionEffect(() => { + useLayoutEffect(() => { pendingNav(); }, [pendingNav]); return ( diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index aade021fdd039..8b17db1dc38a1 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -1201,8 +1201,9 @@ export function hasInstanceAffectedParent( export function startViewTransition( rootContainer: Container, mutationCallback: () => void, - afterMutationCallback: () => void, layoutCallback: () => void, + afterMutationCallback: () => void, + spawnedWorkCallback: () => void, passiveCallback: () => mixed, ): boolean { const ownerDocument: Document = @@ -1213,11 +1214,15 @@ export function startViewTransition( // $FlowFixMe[prop-missing] const transition = ownerDocument.startViewTransition({ update() { - mutationCallback(); - // TODO: Wait for fonts. + // Note: We read the existence of a pending navigation before we apply the + // mutations. That way we're not waiting on a navigation that we spawned + // from this update. Only navigations that started before this commit. const ownerWindow = ownerDocument.defaultView; const pendingNavigation = ownerWindow.navigation && ownerWindow.navigation.transition; + mutationCallback(); + // TODO: Wait for fonts. + layoutCallback(); if (pendingNavigation) { return pendingNavigation.finished.then( afterMutationCallback, @@ -1241,13 +1246,13 @@ export function startViewTransition( console.error( 'A ViewTransition timed out because a Navigation stalled. ' + 'This can happen if a Navigation is blocked on React itself. ' + - "Such as if it's resolved inside useLayoutEffect. " + - 'This can be solved by moving the resolution to useInsertionEffect.', + "Such as if it's resolved inside useEffect. " + + 'This can be solved by moving the resolution to useLayoutEffect.', ); } }); } - transition.ready.then(layoutCallback, layoutCallback); + transition.ready.then(spawnedWorkCallback, spawnedWorkCallback); transition.finished.then(() => { // $FlowFixMe[prop-missing] ownerDocument.__reactViewTransition = null; diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 5687e60637d11..55699f0973b76 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -583,8 +583,9 @@ export function hasInstanceAffectedParent( export function startViewTransition( rootContainer: Container, mutationCallback: () => void, - afterMutationCallback: () => void, layoutCallback: () => void, + afterMutationCallback: () => void, + spawnedWorkCallback: () => void, passiveCallback: () => mixed, ): boolean { return false; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 7a387b690b103..c79d6d541c271 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -637,10 +637,11 @@ const THROTTLED_COMMIT = 2; const NO_PENDING_EFFECTS = 0; const PENDING_MUTATION_PHASE = 1; -const PENDING_AFTER_MUTATION_PHASE = 2; -const PENDING_LAYOUT_PHASE = 3; -const PENDING_PASSIVE_PHASE = 4; -let pendingEffectsStatus: 0 | 1 | 2 | 3 | 4 = 0; +const PENDING_LAYOUT_PHASE = 2; +const PENDING_AFTER_MUTATION_PHASE = 3; +const PENDING_SPAWNED_WORK = 4; +const PENDING_PASSIVE_PHASE = 5; +let pendingEffectsStatus: 0 | 1 | 2 | 3 | 4 | 5 = 0; let pendingEffectsRoot: FiberRoot = (null: any); let pendingFinishedWork: Fiber = (null: any); let pendingEffectsLanes: Lanes = NoLanes; @@ -3432,19 +3433,17 @@ function commitRoot( startViewTransition( root.containerInfo, flushMutationEffects, - flushAfterMutationEffects, flushLayoutEffects, - // TODO: This flushes passive effects at the end of the transition but - // we also schedule work to flush them separately which we really shouldn't. - // We use flushPendingEffects instead of + flushAfterMutationEffects, + flushSpawnedWork, flushPassiveEffects, ); if (!startedViewTransition) { // Flush synchronously. flushMutationEffects(); - // Skip flushAfterMutationEffects - pendingEffectsStatus = PENDING_LAYOUT_PHASE; flushLayoutEffects(); + // Skip flushAfterMutationEffects + flushSpawnedWork(); } } @@ -3457,7 +3456,7 @@ function flushAfterMutationEffects(): void { const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; commitAfterMutationEffects(root, finishedWork, lanes); - pendingEffectsStatus = PENDING_LAYOUT_PHASE; + pendingEffectsStatus = PENDING_SPAWNED_WORK; } function flushMutationEffects(): void { @@ -3503,16 +3502,11 @@ function flushMutationEffects(): void { // componentWillUnmount, but before the layout phase, so that the finished // work is current during componentDidMount/Update. root.current = finishedWork; - pendingEffectsStatus = PENDING_AFTER_MUTATION_PHASE; + pendingEffectsStatus = PENDING_LAYOUT_PHASE; } function flushLayoutEffects(): void { - if ( - pendingEffectsStatus !== PENDING_LAYOUT_PHASE && - // If a startViewTransition times out, we might flush this earlier than - // after mutation phase. In that case, we just skip the after mutation phase. - pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE - ) { + if (pendingEffectsStatus !== PENDING_LAYOUT_PHASE) { return; } pendingEffectsStatus = NO_PENDING_EFFECTS; @@ -3520,10 +3514,6 @@ function flushLayoutEffects(): void { const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; - const completedRenderEndTime = pendingEffectsRenderEndTime; - const recoverableErrors = pendingRecoverableErrors; - const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate; - const suspendedCommitReason = pendingSuspendedCommitReason; const subtreeHasLayoutEffects = (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; @@ -3554,11 +3544,32 @@ function flushLayoutEffects(): void { ReactSharedInternals.T = prevTransition; } } + pendingEffectsStatus = PENDING_AFTER_MUTATION_PHASE; +} + +function flushSpawnedWork(): void { + if ( + pendingEffectsStatus !== PENDING_SPAWNED_WORK && + // If a startViewTransition times out, we might flush this earlier than + // after mutation phase. In that case, we just skip the after mutation phase. + pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE + ) { + return; + } + pendingEffectsStatus = NO_PENDING_EFFECTS; // Tell Scheduler to yield at the end of the frame, so the browser has an // opportunity to paint. requestPaint(); + const root = pendingEffectsRoot; + const finishedWork = pendingFinishedWork; + const lanes = pendingEffectsLanes; + const completedRenderEndTime = pendingEffectsRenderEndTime; + const recoverableErrors = pendingRecoverableErrors; + const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate; + const suspendedCommitReason = pendingSuspendedCommitReason; + if (enableProfilerTimer && enableComponentPerformanceTrack) { recordCommitEndTime(); logCommitPhase( @@ -3795,6 +3806,8 @@ export function flushPendingEffects(wasDelayedCommit?: boolean): boolean { // Returns whether passive effects were flushed. flushMutationEffects(); flushLayoutEffects(); + // Skip flushAfterMutation if we're forcing this early. + flushSpawnedWork(); return flushPassiveEffects(wasDelayedCommit); } diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 15db19c3c96da..c05f59af160f0 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -365,8 +365,9 @@ export function hasInstanceAffectedParent( export function startViewTransition( rootContainer: Container, mutationCallback: () => void, - afterMutationCallback: () => void, layoutCallback: () => void, + afterMutationCallback: () => void, + spawnedWorkCallback: () => void, passiveCallback: () => mixed, ): boolean { return false;