From 8309724cb4a497383cc7b3267483ab5c65dad7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 28 Sep 2025 13:51:35 -0400 Subject: [PATCH] [Fiber][DevTools] Add scheduleRetry to DevTools Hook (#34635) When forcing suspense/error we're doing that by scheduling a sync update on the fiber. Resuspending a Suspense boundary can only happen sync update so that makes sense. Erroring also forces a sync commit. This means that no View Transitions fire. However, unsuspending (and dismissing an error dialog) can be async so the reveal should be able to be async. This adds another hook for scheduling using the Retry lane. That way when you play through a reveal sequence of Suspense boundaries (like playing through the timeline), it'll run the animations that would've ran during a loading sequence. --- .../ReactDevToolsHooksIntegration-test.js | 14 +++++ .../src/__tests__/store-test.js | 2 +- .../src/backend/fiber/renderer.js | 58 +++++++++++++++---- .../src/backend/types.js | 2 + .../src/ReactFiberReconciler.js | 11 ++++ 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js index 9c38c5d832..9e670ec7f7 100644 --- a/packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js @@ -17,6 +17,7 @@ describe('React hooks DevTools integration', () => { let act; let overrideHookState; let scheduleUpdate; + let scheduleRetry; let setSuspenseHandler; let waitForAll; @@ -27,6 +28,7 @@ describe('React hooks DevTools integration', () => { inject: injected => { overrideHookState = injected.overrideHookState; scheduleUpdate = injected.scheduleUpdate; + scheduleRetry = injected.scheduleRetry; setSuspenseHandler = injected.setSuspenseHandler; }, supportsFiber: true, @@ -312,5 +314,17 @@ describe('React hooks DevTools integration', () => { } else { expect(renderer.toJSON().children).toEqual(['Done']); } + + if (scheduleRetry) { + // Lock again, synchronously + setSuspenseHandler(() => true); + await act(() => scheduleUpdate(fiber)); // Re-render + expect(renderer.toJSON().children).toEqual(['Loading']); + + // Release the lock again but this time using retry lane + setSuspenseHandler(() => false); + await act(() => scheduleRetry(fiber)); // Re-render + expect(renderer.toJSON().children).toEqual(['Done']); + } }); }); diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 48a42f7104..9de9b564e9 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -838,7 +838,7 @@ describe('Store', () => { `); - await act(() => + await actAsync(() => agent.overrideSuspense({ id: store.getElementIDAtIndex(2), rendererID, diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index c03b657e60..0dd3084af5 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -1065,6 +1065,7 @@ export function attach( setErrorHandler, setSuspenseHandler, scheduleUpdate, + scheduleRetry, getCurrentFiber, } = renderer; const supportsTogglingError = @@ -7754,7 +7755,13 @@ export function attach( // First override is added. Switch React to slower path. setErrorHandler(shouldErrorFiberAccordingToMap); } - scheduleUpdate(fiber); + if (!forceError && typeof scheduleRetry === 'function') { + // If we're dismissing an error and the renderer supports it, use a Retry instead of Sync + // This would allow View Transitions to proceed as if the error was dismissed using a Transition. + scheduleRetry(fiber); + } else { + scheduleUpdate(fiber); + } } function shouldSuspendFiberAlwaysFalse() { @@ -7812,7 +7819,13 @@ export function attach( setSuspenseHandler(shouldSuspendFiberAlwaysFalse); } } - scheduleUpdate(fiber); + if (!forceFallback && typeof scheduleRetry === 'function') { + // If we're unsuspending and the renderer supports it, use a Retry instead of Sync + // to allow for things like View Transitions to proceed the way they would for real. + scheduleRetry(fiber); + } else { + scheduleUpdate(fiber); + } } /** @@ -7834,11 +7847,10 @@ export function attach( } // TODO: Allow overriding the timeline for the specified root. - forceFallbackForFibers.forEach(fiber => { - scheduleUpdate(fiber); - }); - forceFallbackForFibers.clear(); + const unsuspendedSet: Set = new Set(forceFallbackForFibers); + + let resuspended = false; for (let i = 0; i < suspendedSet.length; ++i) { const instance = idToDevToolsInstanceMap.get(suspendedSet[i]); if (instance === undefined) { @@ -7850,15 +7862,41 @@ export function attach( if (instance.kind === FIBER_INSTANCE) { const fiber = instance.data; - forceFallbackForFibers.add(fiber); - // We could find a minimal set that covers all the Fibers in this suspended set. - // For now we rely on React's batching of updates. - scheduleUpdate(fiber); + if ( + forceFallbackForFibers.has(fiber) || + (fiber.alternate !== null && + forceFallbackForFibers.has(fiber.alternate)) + ) { + // We're already forcing fallback for this fiber. Mark it as not unsuspended. + unsuspendedSet.delete(fiber); + if (fiber.alternate !== null) { + unsuspendedSet.delete(fiber.alternate); + } + } else { + forceFallbackForFibers.add(fiber); + // We could find a minimal set that covers all the Fibers in this suspended set. + // For now we rely on React's batching of updates. + scheduleUpdate(fiber); + resuspended = true; + } } else { console.warn(`Cannot not suspend ID '${suspendedSet[i]}'.`); } } + // Unsuspend any existing forced fallbacks if they're not in the new set. + unsuspendedSet.forEach(fiber => { + forceFallbackForFibers.delete(fiber); + if (!resuspended && typeof scheduleRetry === 'function') { + // If nothing new resuspended we don't need this to be sync. If we're only + // unsuspending then we can schedule this as a Retry if the renderer supports it. + // That way we can trigger animations. + scheduleRetry(fiber); + } else { + scheduleUpdate(fiber); + } + }); + if (forceFallbackForFibers.size > 0) { // First override is added. Switch React to slower path. // TODO: Semantics for suspending a timeline are different. We want a suspended diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 9d3e5a0d04..481bf65e21 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -155,6 +155,8 @@ export type ReactRenderer = { ) => void, // 16.9+ scheduleUpdate?: ?(fiber: Object) => void, + // 19.2+ + scheduleRetry?: ?(fiber: Object) => void, setSuspenseHandler?: ?(shouldSuspend: (fiber: Object) => boolean) => void, // Only injected by React v16.8+ in order to support hooks inspection. currentDispatcherRef?: LegacyDispatcherRef | CurrentDispatcherRef, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 3b4d321416..65eebf04e4 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -98,6 +98,7 @@ import { getHighestPriorityPendingLanes, higherPriorityLane, getBumpedLaneForHydrationByLane, + claimNextRetryLane, } from './ReactFiberLane'; import { scheduleRefresh, @@ -599,6 +600,7 @@ let overrideProps = null; let overridePropsDeletePath = null; let overridePropsRenamePath = null; let scheduleUpdate = null; +let scheduleRetry = null; let setErrorHandler = null; let setSuspenseHandler = null; @@ -835,6 +837,14 @@ if (__DEV__) { } }; + scheduleRetry = (fiber: Fiber) => { + const lane = claimNextRetryLane(); + const root = enqueueConcurrentRenderForLane(fiber, lane); + if (root !== null) { + scheduleUpdateOnFiber(root, fiber, lane); + } + }; + setErrorHandler = (newShouldErrorImpl: Fiber => ?boolean) => { shouldErrorImpl = newShouldErrorImpl; }; @@ -886,6 +896,7 @@ export function injectIntoDevTools(): boolean { internals.overridePropsDeletePath = overridePropsDeletePath; internals.overridePropsRenamePath = overridePropsRenamePath; internals.scheduleUpdate = scheduleUpdate; + internals.scheduleRetry = scheduleRetry; internals.setErrorHandler = setErrorHandler; internals.setSuspenseHandler = setSuspenseHandler; // React Refresh