From 5dd163b49e1c2ba94a23ce3b8c2c1874d3fc1d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 30 Sep 2025 14:37:14 -0400 Subject: [PATCH 1/2] [DevTools] Auto-scroll when stepping through the timeline (#34653) This brings the Suspense boundary that's switching into view so that when you play the loading sequence you can see how it plays out. Otherwise it's really hard to find where things are changing. This assumes we'll also scroll synchronize the suspense tab which will bring it into view there too. --- .../src/backend/fiber/renderer.js | 18 ++ .../src/backend/flight/renderer.js | 3 + .../src/backend/legacy/renderer.js | 3 + .../src/backend/types.js | 11 ++ .../src/backend/views/Highlighter/index.js | 166 ++++++++++++++++-- packages/react-devtools-shared/src/bridge.js | 5 + .../views/SuspenseTab/SuspenseTimeline.js | 18 +- .../src/devtools/views/hooks.js | 27 ++- 8 files changed, 231 insertions(+), 20 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 0dd3084af5aa9..e37a47daf6b6b 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -5651,6 +5651,23 @@ export function attach( } } + function findLastKnownRectsForID(id: number): null | Array { + try { + const devtoolsInstance = idToDevToolsInstanceMap.get(id); + if (devtoolsInstance === undefined) { + console.warn(`Could not find DevToolsInstance with id "${id}"`); + return null; + } + if (devtoolsInstance.suspenseNode === null) { + return null; + } + return devtoolsInstance.suspenseNode.rects; + } catch (err) { + // The fiber might have unmounted by now. + return null; + } + } + function getDisplayNameForElementID(id: number): null | string { const devtoolsInstance = idToDevToolsInstanceMap.get(id); if (devtoolsInstance === undefined) { @@ -8387,6 +8404,7 @@ export function attach( getSerializedElementValueByPath, deletePath, findHostInstancesForElementID, + findLastKnownRectsForID, flushInitialOperations, getBestMatchForTrackedPath, getDisplayNameForElementID, diff --git a/packages/react-devtools-shared/src/backend/flight/renderer.js b/packages/react-devtools-shared/src/backend/flight/renderer.js index 75763b1f18499..d0dc9094334eb 100644 --- a/packages/react-devtools-shared/src/backend/flight/renderer.js +++ b/packages/react-devtools-shared/src/backend/flight/renderer.js @@ -152,6 +152,9 @@ export function attach( findHostInstancesForElementID() { return null; }, + findLastKnownRectsForID() { + return null; + }, flushInitialOperations() {}, getBestMatchForTrackedPath() { return null; diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index b59c0292942c6..8a245155ef2cc 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -1168,6 +1168,9 @@ export function attach( const hostInstance = findHostInstanceForInternalID(id); return hostInstance == null ? null : [hostInstance]; }, + findLastKnownRectsForID() { + return null; + }, getOwnersList, getPathForElement, getProfilingData, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 481bf65e210ed..e002740cb69f7 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -101,6 +101,16 @@ export type FindHostInstancesForElementID = ( id: number, ) => null | $ReadOnlyArray; +type Rect = { + x: number, + y: number, + width: number, + height: number, + ... +}; +export type FindLastKnownRectsForID = ( + id: number, +) => null | $ReadOnlyArray; export type ReactProviderType = { $$typeof: symbol | number, _context: ReactContext, @@ -411,6 +421,7 @@ export type RendererInterface = { path: Array, ) => void, findHostInstancesForElementID: FindHostInstancesForElementID, + findLastKnownRectsForID: FindLastKnownRectsForID, flushInitialOperations: () => void, getBestMatchForTrackedPath: () => PathMatch | null, getComponentStack?: GetComponentStack, diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js index 419fefc4ffcf8..2b651cdc9cc45 100644 --- a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js +++ b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js @@ -11,6 +11,7 @@ import Agent from 'react-devtools-shared/src/backend/agent'; import {hideOverlay, showOverlay} from './Highlighter'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; +import type {RendererInterface} from '../../types'; // This plug-in provides in-page highlighting of the selected element. // It is used by the browser extension and the standalone DevTools shell (when connected to a browser). @@ -25,6 +26,7 @@ export default function setupHighlighter( ): void { bridge.addListener('clearHostInstanceHighlight', clearHostInstanceHighlight); bridge.addListener('highlightHostInstance', highlightHostInstance); + bridge.addListener('scrollToHostInstance', scrollToHostInstance); bridge.addListener('shutdown', stopInspectingHost); bridge.addListener('startInspectingHost', startInspectingHost); bridge.addListener('stopInspectingHost', stopInspectingHost); @@ -111,24 +113,162 @@ export default function setupHighlighter( } const nodes = renderer.findHostInstancesForElementID(id); + if (nodes != null) { + for (let i = 0; i < nodes.length; i++) { + const node = nodes[0]; + if (node === null) { + continue; + } + const nodeRects = + // $FlowFixMe[method-unbinding] + typeof node.getClientRects === 'function' + ? node.getClientRects() + : []; + // If this is currently display: none, then try another node. + // This can happen when one of the host instances is a hoistable. + if ( + nodeRects.length > 0 && + (nodeRects.length > 2 || + nodeRects[0].width > 0 || + nodeRects[0].height > 0) + ) { + // $FlowFixMe[method-unbinding] + if (scrollIntoView && typeof node.scrollIntoView === 'function') { + if (scrollDelayTimer) { + clearTimeout(scrollDelayTimer); + scrollDelayTimer = null; + } + // If the node isn't visible show it before highlighting it. + // We may want to reconsider this; it might be a little disruptive. + node.scrollIntoView({block: 'nearest', inline: 'nearest'}); + } + + showOverlay(nodes, displayName, agent, hideAfterTimeout); + + if (openBuiltinElementsPanel) { + window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node; + bridge.send('syncSelectionToBuiltinElementsPanel'); + } + return; + } + } + } - if (nodes != null && nodes[0] != null) { - const node = nodes[0]; - // $FlowFixMe[method-unbinding] - if (scrollIntoView && typeof node.scrollIntoView === 'function') { - // If the node isn't visible show it before highlighting it. - // We may want to reconsider this; it might be a little disruptive. - node.scrollIntoView({block: 'nearest', inline: 'nearest'}); + hideOverlay(agent); + } + + function attemptScrollToHostInstance( + renderer: RendererInterface, + id: number, + ) { + const nodes = renderer.findHostInstancesForElementID(id); + if (nodes != null) { + for (let i = 0; i < nodes.length; i++) { + const node = nodes[0]; + if (node === null) { + continue; + } + const nodeRects = + // $FlowFixMe[method-unbinding] + typeof node.getClientRects === 'function' + ? node.getClientRects() + : []; + // If this is currently display: none, then try another node. + // This can happen when one of the host instances is a hoistable. + if ( + nodeRects.length > 0 && + (nodeRects.length > 2 || + nodeRects[0].width > 0 || + nodeRects[0].height > 0) + ) { + // $FlowFixMe[method-unbinding] + if (typeof node.scrollIntoView === 'function') { + node.scrollIntoView({ + block: 'nearest', + inline: 'nearest', + behavior: 'smooth', + }); + return true; + } + } } + } + return false; + } + + let scrollDelayTimer = null; + function scrollToHostInstance({ + id, + rendererID, + }: { + id: number, + rendererID: number, + }) { + // Always hide the existing overlay so it doesn't obscure the element. + // If you wanted to show the overlay, highlightHostInstance should be used instead + // with the scrollIntoView option. + hideOverlay(agent); - showOverlay(nodes, displayName, agent, hideAfterTimeout); + if (scrollDelayTimer) { + clearTimeout(scrollDelayTimer); + scrollDelayTimer = null; + } - if (openBuiltinElementsPanel) { - window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node; - bridge.send('syncSelectionToBuiltinElementsPanel'); + const renderer = agent.rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + return; + } + + // In some cases fiber may already be unmounted + if (!renderer.hasElementWithId(id)) { + return; + } + + if (attemptScrollToHostInstance(renderer, id)) { + return; + } + + // It's possible that the current state of a Suspense boundary doesn't have a position + // in the tree. E.g. because it's not yet mounted in the state we're moving to. + // Such as if it's in a null tree or inside another boundary's hidden state. + // In this case we use the last known position and try to scroll to that. + const rects = renderer.findLastKnownRectsForID(id); + if (rects !== null && rects.length > 0) { + let x = Infinity; + let y = Infinity; + for (let i = 0; i < rects.length; i++) { + const rect = rects[i]; + if (rect.x < x) { + x = rect.x; + } + if (rect.y < y) { + y = rect.y; + } } - } else { - hideOverlay(agent); + const element = document.documentElement; + if (!element) { + return; + } + // Check if the target corner is already in the viewport. + if ( + x < window.scrollX || + y < window.scrollY || + x > window.scrollX + element.clientWidth || + y > window.scrollY + element.clientHeight + ) { + window.scrollTo({ + top: y, + left: x, + behavior: 'smooth', + }); + } + // It's possible that after mount, we're able to scroll deeper once the new nodes + // have mounted. Let's try again after mount. Ideally we'd know which commit this + // is going to be but for now we just try after 100ms. + scrollDelayTimer = setTimeout(() => { + attemptScrollToHostInstance(renderer, id); + }, 100); } } diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 616f2d3d3ec23..aa9c867e1f1f2 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -93,6 +93,10 @@ type HighlightHostInstance = { scrollIntoView: boolean, }; +type ScrollToHostInstance = { + ...ElementAndRendererID, +}; + type OverrideValue = { ...ElementAndRendererID, path: Array, @@ -254,6 +258,7 @@ type FrontendEvents = { startInspectingHost: [], startProfiling: [StartProfilingParams], stopInspectingHost: [boolean], + scrollToHostInstance: [ScrollToHostInstance], stopProfiling: [], storeAsGlobal: [StoreAsGlobalParams], updateComponentFilters: [Array], diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js index 18901b45b9ffb..b7340da915b9b 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTimeline.js @@ -8,10 +8,10 @@ */ import * as React from 'react'; -import {useContext, useEffect} from 'react'; +import {useContext, useEffect, useRef} from 'react'; import {BridgeContext, StoreContext} from '../context'; import {TreeDispatcherContext} from '../Components/TreeContext'; -import {useHighlightHostInstance} from '../hooks'; +import {useHighlightHostInstance, useScrollToHostInstance} from '../hooks'; import { SuspenseTreeDispatcherContext, SuspenseTreeStateContext, @@ -28,6 +28,7 @@ function SuspenseTimelineInput() { const suspenseTreeDispatch = useContext(SuspenseTreeDispatcherContext); const {highlightHostInstance, clearHighlightHostInstance} = useHighlightHostInstance(); + const scrollToHostInstance = useScrollToHostInstance(); const { selectedRootID: rootID, @@ -77,7 +78,6 @@ function SuspenseTimelineInput() { function skipPrevious() { const nextSelectedSuspenseID = timeline[timelineIndex - 1]; - highlightHostInstance(nextSelectedSuspenseID); treeDispatch({ type: 'SELECT_ELEMENT_BY_ID', payload: nextSelectedSuspenseID, @@ -90,7 +90,6 @@ function SuspenseTimelineInput() { function skipForward() { const nextSelectedSuspenseID = timeline[timelineIndex + 1]; - highlightHostInstance(nextSelectedSuspenseID); treeDispatch({ type: 'SELECT_ELEMENT_BY_ID', payload: nextSelectedSuspenseID, @@ -108,6 +107,7 @@ function SuspenseTimelineInput() { }); } + const isInitialMount = useRef(true); // TODO: useEffectEvent here once it's supported in all versions DevTools supports. // For now we just exclude it from deps since we don't lint those anyway. function changeTimelineIndex(newIndex: number) { @@ -132,6 +132,16 @@ function SuspenseTimelineInput() { rootID, suspendedSet, }); + if (isInitialMount.current) { + // Skip scrolling on initial mount. Only when we're changing the timeline. + isInitialMount.current = false; + } else { + // When we're scrubbing through the timeline, scroll the current boundary + // into view as it was just revealed. This is after we override the milestone + // to reveal it. + const selectedSuspenseID = timeline[timelineIndex]; + scrollToHostInstance(selectedSuspenseID); + } } useEffect(() => { diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index da69d6b493a19..a4ed2da526e16 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -345,13 +345,13 @@ export function useSubscription({ export function useHighlightHostInstance(): { clearHighlightHostInstance: () => void, - highlightHostInstance: (id: number) => void, + highlightHostInstance: (id: number, scrollIntoView?: boolean) => void, } { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); const highlightHostInstance = useCallback( - (id: number) => { + (id: number, scrollIntoView?: boolean = false) => { const element = store.getElementByID(id); const rendererID = store.getRendererIDForElement(id); if (element !== null && rendererID !== null) { @@ -365,7 +365,7 @@ export function useHighlightHostInstance(): { id, openBuiltinElementsPanel: false, rendererID, - scrollIntoView: false, + scrollIntoView: scrollIntoView, }); } }, @@ -381,3 +381,24 @@ export function useHighlightHostInstance(): { clearHighlightHostInstance, }; } + +export function useScrollToHostInstance(): (id: number) => void { + const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); + + const scrollToHostInstance = useCallback( + (id: number) => { + const element = store.getElementByID(id); + const rendererID = store.getRendererIDForElement(id); + if (element !== null && rendererID !== null) { + bridge.send('scrollToHostInstance', { + id, + rendererID, + }); + } + }, + [store, bridge], + ); + + return scrollToHostInstance; +} From 554a373d7e0202a6f54e28e8258497bdc3377b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 30 Sep 2025 14:37:40 -0400 Subject: [PATCH 2/2] [DevTools] Use the scrollWidth/Height for the root when the root is the documentElement (#34651) If I can scroll the document due to it overflowing, I should be able to scroll the suspense tab as much. The real rect for the root when it's the document is really the full scroll height. This doesn't fully eliminate the need to do recursive bounding boxes for the root since it's still possible to have the rects overflow. E.g. if they're currently resuspended or inside nested scrolls. ~However, maybe we should have the actual paintable root rect just be this rectangle instead of including the recursive ones.~ Actually never mind. The root really represents the Transition so it doesn't make sense to give it any specific rectangle. It's rather the whole background. --- .../src/backend/fiber/renderer.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index e37a47daf6b6b..1a76c85902286 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2251,8 +2251,23 @@ export function attach( } if (typeof instance.getClientRects === 'function') { // DOM - const result: Array = []; const doc = instance.ownerDocument; + if (instance === doc.documentElement) { + // This is the document element. The size of this element is not actually + // what determines the whole scrollable area of the screen. Because any + // thing that overflows the document will also contribute to the scrollable. + // This is unlike overflow: scroll which clips those. + // Therefore, we use the scrollable size for this rect instead. + return [ + { + x: 0, + y: 0, + width: instance.scrollWidth, + height: instance.scrollHeight, + }, + ]; + } + const result: Array = []; const win = doc && doc.defaultView; const scrollX = win ? win.scrollX : 0; const scrollY = win ? win.scrollY : 0;