From f78b2343cc1af0647110b52fcefef8d5abffaaae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 28 Sep 2025 10:15:31 -0400 Subject: [PATCH 1/3] [DevTools] Recursively compute the bounding rect of the roots (#34629) It's possible for the children to overflow the bounding rect of the root in general when they overflow in the DOM. However even when it doesn't overflow in the DOM, the bounding rect of the root can shrink while the content is suspended. In fact, it's very likely. Originally I thought we didn't need to consider this recursively because document scrolling takes absolute positioned content into account but because we're using nested overflow scrolling, we have to manually compute this. --- .../views/SuspenseTab/SuspenseRects.js | 69 +++++++++++++------ 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js index 5dac93a182c1c..d67cc9a9fe3da 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseRects.js @@ -184,6 +184,42 @@ function getBoundingBox(rects: $ReadOnlyArray | null): Rect { }; } +function computeBoundingRectRecursively( + store: Store, + node: SuspenseNode, + bounds: { + minX: number, + minY: number, + maxX: number, + maxY: number, + }, +): void { + const rects = node.rects; + if (rects !== null) { + for (let j = 0; j < rects.length; j++) { + const rect = rects[j]; + if (rect.x < bounds.minX) { + bounds.minX = rect.x; + } + if (rect.x + rect.width > bounds.maxX) { + bounds.maxX = rect.x + rect.width; + } + if (rect.y < bounds.minY) { + bounds.minY = rect.y; + } + if (rect.y + rect.height > bounds.maxY) { + bounds.maxY = rect.y + rect.height; + } + } + } + for (let i = 0; i < node.children.length; i++) { + const child = store.getSuspenseByID(node.children[i]); + if (child !== null) { + computeBoundingRectRecursively(store, child, bounds); + } + } +} + function getDocumentBoundingRect( store: Store, roots: $ReadOnlyArray, @@ -192,10 +228,12 @@ function getDocumentBoundingRect( return {x: 0, y: 0, width: 0, height: 0}; } - let minX = Number.POSITIVE_INFINITY; - let minY = Number.POSITIVE_INFINITY; - let maxX = Number.NEGATIVE_INFINITY; - let maxY = Number.NEGATIVE_INFINITY; + const bounds = { + minX: Number.POSITIVE_INFINITY, + minY: Number.POSITIVE_INFINITY, + maxX: Number.NEGATIVE_INFINITY, + maxY: Number.NEGATIVE_INFINITY, + }; for (let i = 0; i < roots.length; i++) { const rootID = roots[i]; @@ -203,30 +241,19 @@ function getDocumentBoundingRect( if (root === null) { continue; } - - const rects = root.rects; - if (rects === null) { - continue; - } - for (let j = 0; j < rects.length; j++) { - const rect = rects[j]; - minX = Math.min(minX, rect.x); - minY = Math.min(minY, rect.y); - maxX = Math.max(maxX, rect.x + rect.width); - maxY = Math.max(maxY, rect.y + rect.height); - } + computeBoundingRectRecursively(store, root, bounds); } - if (minX === Number.POSITIVE_INFINITY) { + if (bounds.minX === Number.POSITIVE_INFINITY) { // No rects found, return empty rect return {x: 0, y: 0, width: 0, height: 0}; } return { - x: minX, - y: minY, - width: maxX - minX, - height: maxY - minY, + x: bounds.minX, + y: bounds.minY, + width: bounds.maxX - bounds.minX, + height: bounds.maxY - bounds.minY, }; } From 09d3cd8fb55f7d13b9d58495367a4b9660451c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 28 Sep 2025 12:09:08 -0400 Subject: [PATCH 2/3] [DevTools] Larger panel buttons and center (#34619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel icons are quite small. Especially compared to the equivalent buttons elsewhere in Chrome DevTools that otherwise use the same icons. This makes them a little bigger to make them similar size to our other button icons. They were also a bit off center. This centers them as well. Before: Screenshot 2025-09-26 at 4 23 15 PM After: Screenshot 2025-09-26 at 4 22 57 PM --- packages/react-devtools-shared/src/devtools/views/ButtonIcon.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js b/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js index a94766d4f1235..1f3c339608098 100644 --- a/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js +++ b/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js @@ -52,7 +52,7 @@ type Props = { type: IconType, }; -const panelIcons = '0 -960 960 820'; +const panelIcons = '96 -864 768 768'; export default function ButtonIcon({className = '', type}: Props): React.Node { let pathData = null; let viewBox = '0 0 24 24'; 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 3/3] [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 9c38c5d832f7e..9e670ec7f7142 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 48a42f71045b4..9de9b564e93a6 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 c03b657e60f86..0dd3084af5aa9 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 9d3e5a0d04e25..481bf65e210ed 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 3b4d321416b98..65eebf04e486b 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