From c7155bc2d664c0e3931c4364a9f52ef8aa044ecc Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 12 Sep 2025 22:21:04 -0400 Subject: [PATCH 1/3] Track start of Transition --- packages/react-reconciler/src/ReactFiberTransition.js | 2 ++ packages/react-reconciler/src/ReactFiberWorkLoop.js | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberTransition.js b/packages/react-reconciler/src/ReactFiberTransition.js index 6fa0e4359c621..180a09b3659f4 100644 --- a/packages/react-reconciler/src/ReactFiberTransition.js +++ b/packages/react-reconciler/src/ReactFiberTransition.js @@ -28,6 +28,7 @@ import {createCursor, push, pop} from './ReactFiberStack'; import { getWorkInProgressRoot, getWorkInProgressTransitions, + markTransitionStarted, } from './ReactFiberWorkLoop'; import { createCache, @@ -79,6 +80,7 @@ ReactSharedInternals.S = function onStartTransitionFinishForReconciler( transition: Transition, returnValue: mixed, ) { + markTransitionStarted(); if ( typeof returnValue === 'object' && returnValue !== null && diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 00c8ce91c1a75..37023a0c12a32 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -475,6 +475,11 @@ let didIncludeCommitPhaseUpdate: boolean = false; // content as it streams in, to minimize jank. // TODO: Think of a better name for this variable? let globalMostRecentFallbackTime: number = 0; +// Track the most recent time we started a new Transition. This lets us apply +// heuristics like the suspensey image timeout based on how long we've waited +// already. +let globalMostRecentTransitionTime: number = 0; + const FALLBACK_THROTTLE_MS: number = 300; // The absolute time for when we should start giving up on rendering @@ -2284,6 +2289,10 @@ export function markRenderDerivedCause(fiber: Fiber): void { } } +export function markTransitionStarted() { + globalMostRecentTransitionTime = now(); +} + export function markCommitTimeOfFallback() { globalMostRecentFallbackTime = now(); } From d1afba998c255ec8b8f73a04e9442cc67e317026 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 12 Sep 2025 22:29:25 -0400 Subject: [PATCH 2/3] Use the start time of the transition or the previous fallback as the basis for timeouts If some has already elapsed we should wait less. --- packages/react-art/src/ReactFiberConfigART.js | 2 +- .../src/client/ReactFiberConfigDOM.js | 6 ++++-- .../src/ReactFiberConfigFabric.js | 2 +- .../src/ReactFiberConfigNative.js | 2 +- packages/react-noop-renderer/src/createReactNoop.js | 6 +++--- packages/react-reconciler/src/ReactFiberWorkLoop.js | 10 +++++++++- .../__tests__/ReactFiberHostContext-test.internal.js | 2 +- .../src/ReactFiberConfigTestHost.js | 2 +- 8 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index be0f1210974de..0dc7dd6bb1543 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -615,7 +615,7 @@ export function suspendInstance(instance, type, props) {} export function suspendOnActiveViewTransition(container) {} -export function waitForCommitToBeReady() { +export function waitForCommitToBeReady(timeoutOffset) { return null; } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 6b71052d9ae6e..105cbe3a0e50c 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -6067,7 +6067,9 @@ export function suspendOnActiveViewTransition(rootContainer: Container): void { activeViewTransition.finished.then(ping, ping); } -export function waitForCommitToBeReady(): null | ((() => void) => () => void) { +export function waitForCommitToBeReady( + timeoutOffset: number, +): null | ((() => void) => () => void) { if (suspendedState === null) { throw new Error( 'Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug.', @@ -6102,7 +6104,7 @@ export function waitForCommitToBeReady(): null | ((() => void) => () => void) { state.unsuspend = null; unsuspend(); } - }, 60000); // one minute + }, 60000 + timeoutOffset); // one minute state.unsuspend = commit; diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 5ad58533624c1..e97deb0738ac9 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -612,7 +612,7 @@ export function suspendInstance( export function suspendOnActiveViewTransition(container: Container): void {} -export function waitForCommitToBeReady(): null { +export function waitForCommitToBeReady(timeoutOffset: number): null { return null; } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 18a346495f640..bb79fb319edfc 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -788,7 +788,7 @@ export function suspendInstance( export function suspendOnActiveViewTransition(container: Container): void {} -export function waitForCommitToBeReady(): null { +export function waitForCommitToBeReady(timeoutOffset: number): null { return null; } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 83cbb24744a9c..7e5a8db5a6623 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -361,9 +361,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { } } - function waitForCommitToBeReady(): - | ((commit: () => mixed) => () => void) - | null { + function waitForCommitToBeReady( + timeoutOffset: number, + ): ((commit: () => mixed) => () => void) | null { const subscription = suspenseyCommitSubscription; if (subscription !== null) { suspenseyCommitSubscription = null; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 37023a0c12a32..f8a3be658b9e2 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1505,10 +1505,18 @@ function commitRootWhenReady( suspendOnActiveViewTransition(root.containerInfo); } } + // For timeouts we use the previous fallback commit for retries and + // the start time of the transition for transitions. This offset + // represents the time already passed. + const timeoutOffset = includesOnlyRetries(lanes) + ? globalMostRecentFallbackTime - now() + : includesOnlyTransitions(lanes) + ? globalMostRecentTransitionTime - now() + : 0; // At the end, ask the renderer if it's ready to commit, or if we should // suspend. If it's not ready, it will return a callback to subscribe to // a ready event. - const schedulePendingCommit = waitForCommitToBeReady(); + const schedulePendingCommit = waitForCommitToBeReady(timeoutOffset); if (schedulePendingCommit !== null) { // NOTE: waitForCommitToBeReady returns a subscribe function so that we // only allocate a function if the commit isn't ready yet. The other diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 2351f7ed01b8f..099f6cfa36bb4 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -109,7 +109,7 @@ describe('ReactFiberHostContext', () => { startSuspendingCommit() {}, suspendInstance(instance, type, props) {}, suspendOnActiveViewTransition(container) {}, - waitForCommitToBeReady() { + waitForCommitToBeReady(timeoutOffset: number) { return null; }, supportsMutation: true, diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index c0fed8ca9bbb0..26bc31a407ef6 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -571,7 +571,7 @@ export function suspendInstance( export function suspendOnActiveViewTransition(container: Container): void {} -export function waitForCommitToBeReady(): null { +export function waitForCommitToBeReady(timeoutOffset: number): null { return null; } From 26e3c427c59bcd0ae55f384b68d2e7addce832cd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 12 Sep 2025 23:12:13 -0400 Subject: [PATCH 3/3] Move the image timeout to a central place like the stylesheet timeout --- .../src/client/ReactFiberConfigDOM.js | 64 +++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 105cbe3a0e50c..72c061e09e565 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -5905,7 +5905,9 @@ export function preloadResource(resource: Resource): boolean { type SuspendedState = { stylesheets: null | Map, - count: number, + count: number, // suspensey css and active view transitions + imgCount: number, // suspensey images + waitingForImages: boolean, // false when we're no longer blocking on images unsuspend: null | (() => void), }; let suspendedState: null | SuspendedState = null; @@ -5914,6 +5916,8 @@ export function startSuspendingCommit(): void { suspendedState = { stylesheets: null, count: 0, + imgCount: 0, + waitingForImages: true, // We use a noop function when we begin suspending because if possible we want the // waitfor step to finish synchronously. If it doesn't we'll return a function to // provide the actual unsuspend function and that will get completed when the count @@ -5922,6 +5926,8 @@ export function startSuspendingCommit(): void { }; } +const SUSPENSEY_STYLESHEET_TIMEOUT = 60000; + const SUSPENSEY_IMAGE_TIMEOUT = 500; export function suspendInstance( @@ -5946,13 +5952,10 @@ export function suspendInstance( // If this browser supports decode() API, we use it to suspend waiting on the image. // The loading should have already started at this point, so it should be enough to // just call decode() which should also wait for the data to finish loading. - state.count++; - const ping = onUnsuspend.bind(state); - Promise.race([ - // $FlowFixMe[prop-missing] - instance.decode(), - new Promise(resolve => setTimeout(resolve, SUSPENSEY_IMAGE_TIMEOUT)), - ]).then(ping, ping); + state.imgCount++; + const ping = onUnsuspendImg.bind(state); + // $FlowFixMe[prop-missing] + instance.decode().then(ping, ping); } } @@ -6087,7 +6090,7 @@ export function waitForCommitToBeReady( // We need to check the count again because the inserted stylesheets may have led to new // tasks to wait on. - if (state.count > 0) { + if (state.count > 0 || state.imgCount > 0) { return commit => { // We almost never want to show content before its styles have loaded. But // eventually we will give up and allow unstyled content. So this number is @@ -6104,37 +6107,62 @@ export function waitForCommitToBeReady( state.unsuspend = null; unsuspend(); } - }, 60000 + timeoutOffset); // one minute + }, SUSPENSEY_STYLESHEET_TIMEOUT + timeoutOffset); + + const imgTimer = setTimeout(() => { + // We're no longer blocked on images. If CSS resolves after this we can commit. + state.waitingForImages = false; + if (state.count === 0) { + if (state.stylesheets) { + insertSuspendedStylesheets(state, state.stylesheets); + } + if (state.unsuspend) { + const unsuspend = state.unsuspend; + state.unsuspend = null; + unsuspend(); + } + } + }, SUSPENSEY_IMAGE_TIMEOUT + timeoutOffset); state.unsuspend = commit; return () => { state.unsuspend = null; clearTimeout(stylesheetTimer); + clearTimeout(imgTimer); }; }; } return null; } -function onUnsuspend(this: SuspendedState) { - this.count--; - if (this.count === 0) { - if (this.stylesheets) { +function checkIfFullyUnsuspended(state: SuspendedState) { + if (state.count === 0 && (state.imgCount === 0 || !state.waitingForImages)) { + if (state.stylesheets) { // If we haven't actually inserted the stylesheets yet we need to do so now before starting the commit. // The reason we do this after everything else has finished is because we want to have all the stylesheets // load synchronously right before mutating. Ideally the new styles will cause a single recalc only on the // new tree. When we filled up stylesheets we only inlcuded stylesheets with matching media attributes so we // wait for them to load before actually continuing. We expect this to increase the count above zero - insertSuspendedStylesheets(this, this.stylesheets); - } else if (this.unsuspend) { - const unsuspend = this.unsuspend; - this.unsuspend = null; + insertSuspendedStylesheets(state, state.stylesheets); + } else if (state.unsuspend) { + const unsuspend = state.unsuspend; + state.unsuspend = null; unsuspend(); } } } +function onUnsuspend(this: SuspendedState) { + this.count--; + checkIfFullyUnsuspended(this); +} + +function onUnsuspendImg(this: SuspendedState) { + this.imgCount--; + checkIfFullyUnsuspended(this); +} + // We use a value that is type distinct from precedence to track which one is last. // This ensures there is no collision with user defined precedences. Normally we would // just track this in module scope but since the precedences are tracked per HoistableRoot