From a96a0f3903ea0a9d45ff7c30a3fd9efe830c4628 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 15 Aug 2025 12:14:23 -0400 Subject: [PATCH 1/4] Fix fragmentInstance#compareDocumentPosition nesting and portal cases (#34069) Found a couple of issues while integrating FragmentInstance#compareDocumentPosition into Fabric. 1. Basic checks of nested host instances were inaccurate. For example, checking the first child of the first child of the Fragment would not return CONTAINED_BY. 2. Then fixing that logic exposed issues with Portals. The DOM positioning relied on the assumption that the first and last top-level children were in the same order as the Fiber tree. I added additional checks against the parent's position in the DOM, and special cased a portaled Fragment by getting its DOM parent from the child instance, rather than taking the instance from the Fiber return. This should be accurate in more cases. Though its still a guess and I'm not sure yet I've covered every variation of this. Portals are hard to deal with and we may end up having to push more results towards IMPLEMENTATION_SPECIFIC if accuracy is an issue. --- .../src/client/ReactFiberConfigDOM.js | 72 +++++++-- .../__tests__/ReactDOMFragmentRefs-test.js | 139 +++++++++++++++--- .../src/ReactFiberTreeReflection.js | 52 +++++-- 3 files changed, 219 insertions(+), 44 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 633adc3db68d8..03a6104e68db5 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -38,9 +38,16 @@ import hasOwnProperty from 'shared/hasOwnProperty'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; import { - isFiberContainedBy, + isFiberContainedByFragment, isFiberFollowing, isFiberPreceding, + isFragmentContainedByFiber, + traverseFragmentInstance, + getFragmentParentHostFiber, + getNextSiblingHostFiber, + getInstanceFromHostFiber, + traverseFragmentInstanceDeeply, + fiberIsPortaledIntoHost, } from 'react-reconciler/src/ReactFiberTreeReflection'; export { @@ -63,13 +70,6 @@ import { markNodeAsHoistable, isOwnedInstance, } from './ReactDOMComponentTree'; -import { - traverseFragmentInstance, - getFragmentParentHostFiber, - getNextSiblingHostFiber, - getInstanceFromHostFiber, - traverseFragmentInstanceDeeply, -} from 'react-reconciler/src/ReactFiberTreeReflection'; export {detachDeletedInstance}; import {hasRole} from './DOMAccessibilityRoles'; @@ -3052,13 +3052,13 @@ FragmentInstance.prototype.compareDocumentPosition = function ( } const children: Array = []; traverseFragmentInstance(this._fragmentFiber, collectChildren, children); + const parentHostInstance = + getInstanceFromHostFiber(parentHostFiber); let result = Node.DOCUMENT_POSITION_DISCONNECTED; if (children.length === 0) { // If the fragment has no children, we can use the parent and // siblings to determine a position. - const parentHostInstance = - getInstanceFromHostFiber(parentHostFiber); const parentResult = parentHostInstance.compareDocumentPosition(otherNode); result = parentResult; if (parentHostInstance === otherNode) { @@ -3095,15 +3095,53 @@ FragmentInstance.prototype.compareDocumentPosition = function ( const lastElement = getInstanceFromHostFiber( children[children.length - 1], ); + + // If the fragment has been portaled into another host instance, we need to + // our best guess is to use the parent of the child instance, rather than + // the fiber tree host parent. + const parentHostInstanceFromDOM = fiberIsPortaledIntoHost(this._fragmentFiber) + ? (getInstanceFromHostFiber(children[0]).parentElement: ?Instance) + : parentHostInstance; + + if (parentHostInstanceFromDOM == null) { + return Node.DOCUMENT_POSITION_DISCONNECTED; + } + + // Check if first and last element are actually in the expected document position + // before relying on them as source of truth for other contained elements + const firstElementIsContained = + parentHostInstanceFromDOM.compareDocumentPosition(firstElement) & + Node.DOCUMENT_POSITION_CONTAINED_BY; + const lastElementIsContained = + parentHostInstanceFromDOM.compareDocumentPosition(lastElement) & + Node.DOCUMENT_POSITION_CONTAINED_BY; const firstResult = firstElement.compareDocumentPosition(otherNode); const lastResult = lastElement.compareDocumentPosition(otherNode); + + const otherNodeIsFirstOrLastChild = + (firstElementIsContained && firstElement === otherNode) || + (lastElementIsContained && lastElement === otherNode); + const otherNodeIsFirstOrLastChildDisconnected = + (!firstElementIsContained && firstElement === otherNode) || + (!lastElementIsContained && lastElement === otherNode); + const otherNodeIsWithinFirstOrLastChild = + firstResult & Node.DOCUMENT_POSITION_CONTAINED_BY || + lastResult & Node.DOCUMENT_POSITION_CONTAINED_BY; + const otherNodeIsBetweenFirstAndLastChildren = + firstElementIsContained && + lastElementIsContained && + firstResult & Node.DOCUMENT_POSITION_FOLLOWING && + lastResult & Node.DOCUMENT_POSITION_PRECEDING; + if ( - (firstResult & Node.DOCUMENT_POSITION_FOLLOWING && - lastResult & Node.DOCUMENT_POSITION_PRECEDING) || - otherNode === firstElement || - otherNode === lastElement + otherNodeIsFirstOrLastChild || + otherNodeIsWithinFirstOrLastChild || + otherNodeIsBetweenFirstAndLastChildren ) { result = Node.DOCUMENT_POSITION_CONTAINED_BY; + } else if (otherNodeIsFirstOrLastChildDisconnected) { + // otherNode has been portaled into another container + result = Node.DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC; } else { result = firstResult; } @@ -3141,7 +3179,9 @@ function validateDocumentPositionWithFiberTree( ): boolean { const otherFiber = getClosestInstanceFromNode(otherNode); if (documentPosition & Node.DOCUMENT_POSITION_CONTAINED_BY) { - return !!otherFiber && isFiberContainedBy(fragmentFiber, otherFiber); + return ( + !!otherFiber && isFiberContainedByFragment(otherFiber, fragmentFiber) + ); } if (documentPosition & Node.DOCUMENT_POSITION_CONTAINS) { if (otherFiber === null) { @@ -3149,7 +3189,7 @@ function validateDocumentPositionWithFiberTree( const ownerDocument = otherNode.ownerDocument; return otherNode === ownerDocument || otherNode === ownerDocument.body; } - return isFiberContainedBy(otherFiber, fragmentFiber); + return isFragmentContainedByFiber(fragmentFiber, otherFiber); } if (documentPosition & Node.DOCUMENT_POSITION_PRECEDING) { return ( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index e7e1d053a7a3e..521792d62e95f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1197,14 +1197,14 @@ describe('FragmentRefs', () => { function Test() { return ( -
-
+
+
-
-
-
+
+
+
-
+
); } @@ -1289,7 +1289,7 @@ describe('FragmentRefs', () => { }, ); - // containerRef preceds and contains the fragment + // containerRef precedes and contains the fragment expectPosition( fragmentRef.current.compareDocumentPosition(containerRef.current), { @@ -1328,7 +1328,7 @@ describe('FragmentRefs', () => { function Test() { return (
-
+
@@ -1491,6 +1491,77 @@ describe('FragmentRefs', () => { ); }); + // @gate enableFragmentRefs + it('handles nested children', async () => { + const fragmentRef = React.createRef(); + const nestedFragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const childCRef = React.createRef(); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + + function Child() { + return ( +
+ C +
+ ); + } + + function Test() { + return ( + +
+ A +
+ +
+ B +
+ + + + ); + } + + await act(() => root.render()); + + expectPosition( + fragmentRef.current.compareDocumentPosition(childARef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childBRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childCRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + }); + // @gate enableFragmentRefs it('returns disconnected for comparison with an unmounted fragment instance', async () => { const fragmentRef = React.createRef(); @@ -1551,11 +1622,11 @@ describe('FragmentRefs', () => { function Test() { return ( -
- {createPortal(
, document.body)} +
+ {createPortal(
, container)} - {createPortal(
, document.body)} -
+ {createPortal(
, container)} +
); @@ -1600,6 +1671,8 @@ describe('FragmentRefs', () => { const childARef = React.createRef(); const childBRef = React.createRef(); const childCRef = React.createRef(); + const childDRef = React.createRef(); + const childERef = React.createRef(); function Test() { const [c, setC] = React.useState(false); @@ -1612,23 +1685,30 @@ describe('FragmentRefs', () => { {createPortal(
- {c ?
: null} + {c ? ( +
+
+
+ ) : null} , document.body, )} {createPortal(

, document.body)} +

); } await act(() => root.render()); - // Due to effect, order is A->B->C - expect(document.body.innerHTML).toBe( - '
' + + // Due to effect, order is E / A->B->C->D + expect(document.body.outerHTML).toBe( + '' + + '
' + '
' + '

' + - '
', + '
' + + '', ); expectPosition( @@ -1642,7 +1722,6 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); - expectPosition( fragmentRef.current.compareDocumentPosition(childARef.current), { @@ -1654,6 +1733,7 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); + // Contained by in DOM, but following in React tree expectPosition( fragmentRef.current.compareDocumentPosition(childBRef.current), { @@ -1676,6 +1756,29 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childDRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + // Preceding DOM but following in React tree + expectPosition( + fragmentRef.current.compareDocumentPosition(childERef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); }); // @gate enableFragmentRefs diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index 45da707dfea23..49a357adf8268 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -26,6 +26,7 @@ import { ActivityComponent, SuspenseComponent, OffscreenComponent, + Fragment, } from './ReactWorkTags'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; @@ -405,6 +406,21 @@ export function getFragmentParentHostFiber(fiber: Fiber): null | Fiber { return null; } +export function fiberIsPortaledIntoHost(fiber: Fiber): boolean { + let foundPortalParent = false; + let parent = fiber.return; + while (parent !== null) { + if (parent.tag === HostPortal) { + foundPortalParent = true; + } + if (parent.tag === HostRoot || parent.tag === HostComponent) { + break; + } + parent = parent.return; + } + return foundPortalParent; +} + export function getInstanceFromHostFiber(fiber: Fiber): I { switch (fiber.tag) { case HostComponent: @@ -443,22 +459,38 @@ function findNextSibling(child: Fiber): boolean { return true; } -export function isFiberContainedBy( - maybeChild: Fiber, - maybeParent: Fiber, +export function isFiberContainedByFragment( + fiber: Fiber, + fragmentFiber: Fiber, ): boolean { - let parent = maybeParent.return; - if (parent === maybeChild || parent === maybeChild.alternate) { - return true; + let current: Fiber | null = fiber; + while (current !== null) { + if ( + current.tag === Fragment && + (current === fragmentFiber || current.alternate === fragmentFiber) + ) { + return true; + } + current = current.return; } - while (parent !== null && parent !== maybeChild) { + return false; +} + +export function isFragmentContainedByFiber( + fragmentFiber: Fiber, + otherFiber: Fiber, +): boolean { + let current: Fiber | null = fragmentFiber; + const fiberHostParent: Fiber | null = + getFragmentParentHostFiber(fragmentFiber); + while (current !== null) { if ( - (parent.tag === HostComponent || parent.tag === HostRoot) && - (parent.return === maybeChild || parent.return === maybeChild.alternate) + (current.tag === HostComponent || current.tag === HostRoot) && + (current === fiberHostParent || current.alternate === fiberHostParent) ) { return true; } - parent = parent.return; + current = current.return; } return false; } From 2ba7b07ce10448cc37d793a50d5ca0999e63aad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 15 Aug 2025 13:34:07 -0400 Subject: [PATCH 2/4] [DevTools] Compute a min and max range for the currently selected suspense boundary (#34201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This computes a min and max range for the whole suspense boundary even when selecting a single component so that each component in a boundary has a consistent range. The start of this range is the earliest start of I/O in that boundary or the end of the previous suspense boundary, whatever is earlier. If the end of the previous boundary would make the range large, then we cap it since it's likely that the other boundary was just an independent render. The end of the range is the latest end of I/O in that boundary. If this is smaller than the end of the previous boundary plus the 300ms throttle, then we extend the end. This visualizes what throttling could potentially do if the previous boundary committed right at its end. Ofc, it might not have committed exactly at that time in this render. So this is just showing a potential throttle that could happen. To see actual throttle, you look in the Performance Track. Screenshot 2025-08-14 at 12 41 43 AM We could come up with some annotation to highlight that this is eligible to be throttled in this case. If the lines don't extend to the edge, then it's likely it was throttled. --- .../src/backend/fiber/renderer.js | 71 +++++++++++++++++++ .../src/backend/legacy/renderer.js | 1 + .../src/backend/types.js | 1 + .../react-devtools-shared/src/backendAPI.js | 2 + .../Components/InspectedElementSuspendedBy.js | 7 +- .../src/frontend/types.js | 2 + 6 files changed, 83 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 1eee4a3ad8a1c..b927b1a58fd7f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -5268,6 +5268,18 @@ export function attach( } } + function getNearestSuspenseNode(instance: DevToolsInstance): SuspenseNode { + while (instance.suspenseNode === null) { + if (instance.parent === null) { + throw new Error( + 'There should always be a SuspenseNode parent on a mounted instance.', + ); + } + instance = instance.parent; + } + return instance.suspenseNode; + } + function getNearestMountedDOMNode(publicInstance: Element): null | Element { let domNode: null | Element = publicInstance; while (domNode && !publicInstanceToDevToolsInstanceMap.has(domNode)) { @@ -5556,6 +5568,56 @@ export function attach( return result; } + const FALLBACK_THROTTLE_MS: number = 300; + + function getSuspendedByRange( + suspenseNode: SuspenseNode, + ): null | [number, number] { + let min = Infinity; + let max = -Infinity; + suspenseNode.suspendedBy.forEach((_, ioInfo) => { + if (ioInfo.end > max) { + max = ioInfo.end; + } + if (ioInfo.start < min) { + min = ioInfo.start; + } + }); + const parentSuspenseNode = suspenseNode.parent; + if (parentSuspenseNode !== null) { + let parentMax = -Infinity; + parentSuspenseNode.suspendedBy.forEach((_, ioInfo) => { + if (ioInfo.end > parentMax) { + parentMax = ioInfo.end; + } + }); + // The parent max is theoretically the earlier the parent could've committed. + // Therefore, the theoretical max that the child could be throttled is that plus 300ms. + const throttleTime = parentMax + FALLBACK_THROTTLE_MS; + if (throttleTime > max) { + // If the theoretical throttle time is later than the earliest reveal then we extend + // the max time to show that this is timespan could possibly get throttled. + max = throttleTime; + } + + // We use the end of the previous boundary as the start time for this boundary unless, + // that's earlier than we'd need to expand to the full fallback throttle range. It + // suggests that the parent was loaded earlier than this one. + let startTime = max - FALLBACK_THROTTLE_MS; + if (parentMax > startTime) { + startTime = parentMax; + } + // If the first fetch of this boundary starts before that, then we use that as the start. + if (startTime < min) { + min = startTime; + } + } + if (min < Infinity && max > -Infinity) { + return [min, max]; + } + return null; + } + function getAwaitStackFromHooks( hooks: HooksTree, asyncInfo: ReactAsyncInfo, @@ -6024,6 +6086,10 @@ export function attach( : fiberInstance.suspendedBy.map(info => serializeAsyncInfo(info, fiberInstance, hooks), ); + const suspendedByRange = getSuspendedByRange( + getNearestSuspenseNode(fiberInstance), + ); + return { id: fiberInstance.id, @@ -6086,6 +6152,7 @@ export function attach( : Array.from(componentLogsEntry.warnings.entries()), suspendedBy: suspendedBy, + suspendedByRange: suspendedByRange, // List of owners owners, @@ -6144,6 +6211,9 @@ export function attach( // Things that Suspended this Server Component (use(), awaits and direct child promises) const suspendedBy = virtualInstance.suspendedBy; + const suspendedByRange = getSuspendedByRange( + getNearestSuspenseNode(virtualInstance), + ); return { id: virtualInstance.id, @@ -6196,6 +6266,7 @@ export function attach( : suspendedBy.map(info => serializeAsyncInfo(info, virtualInstance, null), ), + suspendedByRange: suspendedByRange, // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index c2c278393602a..82a3a8d1905ba 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -858,6 +858,7 @@ export function attach( // Not supported in legacy renderers. suspendedBy: [], + suspendedByRange: null, // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 55a1bc6532e22..0d2a86beb5b8b 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -300,6 +300,7 @@ export type InspectedElement = { // Things that suspended this Instances suspendedBy: Object, // DehydratedData or Array + suspendedByRange: null | [number, number], // List of owners owners: Array | null, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index db22606377da1..dcaec7dc0341c 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -270,6 +270,7 @@ export function convertInspectedElementBackendToFrontend( errors, warnings, suspendedBy, + suspendedByRange, nativeTag, } = inspectedElementBackend; @@ -313,6 +314,7 @@ export function convertInspectedElementBackendToFrontend( hydratedSuspendedBy == null // backwards compat ? [] : hydratedSuspendedBy.map(backendToFrontendSerializedAsyncInfo), + suspendedByRange, nativeTag, }; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js index e5e094955887a..3527c23b06698 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -292,7 +292,7 @@ export default function InspectedElementSuspendedBy({ inspectedElement, store, }: Props): React.Node { - const {suspendedBy} = inspectedElement; + const {suspendedBy, suspendedByRange} = inspectedElement; // Skip the section if nothing suspended this component. if (suspendedBy == null || suspendedBy.length === 0) { @@ -306,6 +306,11 @@ export default function InspectedElementSuspendedBy({ let minTime = Infinity; let maxTime = -Infinity; + if (suspendedByRange !== null) { + // The range of the whole suspense boundary. + minTime = suspendedByRange[0]; + maxTime = suspendedByRange[1]; + } for (let i = 0; i < suspendedBy.length; i++) { const asyncInfo: SerializedAsyncInfo = suspendedBy[i]; if (asyncInfo.awaited.start < minTime) { diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 4c61a8b1e9d71..7c7487b7bd6d0 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -279,6 +279,8 @@ export type InspectedElement = { // Things that suspended this Instances suspendedBy: Object, + // Minimum start time to maximum end time + a potential (not actual) throttle, within the nearest boundary. + suspendedByRange: null | [number, number], // List of owners owners: Array | null, From 2d98b45d92ee9adc5ec6dbd2a5a270e0fd2607a4 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Fri, 15 Aug 2025 19:39:59 +0200 Subject: [PATCH 3/4] [DevTools] Fix Suspense boundaries always being marked as not suspended (#34206) --- .../react-devtools-shared/src/backend/fiber/renderer.js | 8 ++++++++ .../react-devtools-shared/src/backend/legacy/renderer.js | 1 + packages/react-devtools-shared/src/backend/types.js | 2 ++ packages/react-devtools-shared/src/backendAPI.js | 2 ++ .../src/devtools/views/Components/InspectedElement.js | 2 +- .../views/Components/InspectedElementSuspenseToggle.js | 4 +--- packages/react-devtools-shared/src/frontend/types.js | 2 ++ 7 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index b927b1a58fd7f..5e9bcddbfae5f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -6071,6 +6071,11 @@ export function attach( nativeTag = getNativeTag(fiber.stateNode); } + let isSuspended: boolean | null = null; + if (tag === SuspenseComponent) { + isSuspended = memoizedState !== null; + } + const suspendedBy = fiberInstance.suspenseNode !== null ? // If this is a Suspense boundary, then we include everything in the subtree that might suspend @@ -6121,6 +6126,7 @@ export function attach( forceFallbackForFibers.has(fiber) || (fiber.alternate !== null && forceFallbackForFibers.has(fiber.alternate))), + isSuspended: isSuspended, source, @@ -6209,6 +6215,7 @@ export function attach( const componentLogsEntry = componentInfoToComponentLogsMap.get(componentInfo); + const isSuspended = null; // Things that Suspended this Server Component (use(), awaits and direct child promises) const suspendedBy = virtualInstance.suspendedBy; const suspendedByRange = getSuspendedByRange( @@ -6230,6 +6237,7 @@ export function attach( isErrored: false, canToggleSuspense: supportsTogglingSuspense && hasSuspenseBoundary, + isSuspended: isSuspended, source, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 82a3a8d1905ba..592238934f5d3 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -836,6 +836,7 @@ export function attach( // Suspense did not exist in legacy versions canToggleSuspense: false, + isSuspended: null, source: null, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 0d2a86beb5b8b..978b546b8353f 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -285,6 +285,8 @@ export type InspectedElement = { // Is this Suspense, and can its value be overridden now? canToggleSuspense: boolean, + // If this Element is suspended. Currently only set on Suspense boundaries. + isSuspended: boolean | null, // Does the component have legacy context attached to it. hasLegacyContext: boolean, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index dcaec7dc0341c..eb1b6f6df3ff0 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -251,6 +251,7 @@ export function convertInspectedElementBackendToFrontend( canToggleError, isErrored, canToggleSuspense, + isSuspended, hasLegacyContext, id, type, @@ -287,6 +288,7 @@ export function convertInspectedElementBackendToFrontend( canToggleError, isErrored, canToggleSuspense, + isSuspended, hasLegacyContext, id, key, diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js index fd068c0ad1856..8f1d1cd7586a9 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -113,7 +113,7 @@ export default function InspectedElementWrapper(_: Props): React.Node { element !== null && element.type === ElementTypeSuspense && inspectedElement != null && - inspectedElement.state != null; + inspectedElement.isSuspended; const canToggleError = !hideToggleErrorAction && diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspenseToggle.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspenseToggle.js index 3b445505cbcc9..ebb4b5bcd5439 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspenseToggle.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspenseToggle.js @@ -30,15 +30,13 @@ export default function InspectedElementSuspenseToggle({ }: Props): React.Node { const {readOnly} = React.useContext(OptionsContext); - const {id, state, type} = inspectedElement; + const {id, isSuspended, type} = inspectedElement; const canToggleSuspense = !readOnly && inspectedElement.canToggleSuspense; if (type !== ElementTypeSuspense) { return null; } - const isSuspended = state !== null; - const toggleSuspense = (path: any, value: boolean) => { const rendererID = store.getRendererIDForElement(id); if (rendererID !== null) { diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 7c7487b7bd6d0..d7d22b9530c9c 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -264,6 +264,8 @@ export type InspectedElement = { // Is this Suspense, and can its value be overridden now? canToggleSuspense: boolean, + // If this Element is suspended. Currently only set on Suspense boundaries. + isSuspended: boolean | null, // Does the component have legacy context attached to it. hasLegacyContext: boolean, From 8dba9311e581c005af40ca0986155b85e959ae83 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Fri, 15 Aug 2025 19:40:35 +0200 Subject: [PATCH 4/4] [DevTools] Handle fallback unmount in Suspense update path (#34199) --- .../src/__tests__/store-test.js | 79 +++++++++++++++++++ .../src/backend/fiber/renderer.js | 25 +++--- 2 files changed, 94 insertions(+), 10 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 87524ffd045b4..a3a428a9d98d9 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -2696,4 +2696,83 @@ describe('Store', () => { `); }); + + // @reactVersion >= 18.0 + it('can reconcile Suspense in fallback positions', async () => { + let resolveFallback; + const fallbackPromise = new Promise(resolve => { + resolveFallback = resolve; + }); + let resolveContent; + const contentPromise = new Promise(resolve => { + resolveContent = resolve; + }); + + function Component({children, promise}) { + if (promise) { + React.use(promise); + } + return
{children}
; + } + + await actAsync(() => + render( + + Loading fallback... + + }> + + Loading... + + + }> + + done + + , + ), + ); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + [shell] + + + `); + + await actAsync(() => { + resolveFallback(); + }); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + [shell] + + + `); + + await actAsync(() => { + resolveContent(); + }); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + [shell] + + `); + }); }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 5e9bcddbfae5f..439d522801a32 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4736,26 +4736,30 @@ export function attach( ); shouldMeasureSuspenseNode = false; - if (nextFallbackFiber !== null) { + if (prevFallbackFiber !== null || nextFallbackFiber !== null) { const fallbackStashedSuspenseParent = reconcilingParentSuspenseNode; const fallbackStashedSuspensePrevious = previouslyReconciledSiblingSuspenseNode; const fallbackStashedSuspenseRemaining = remainingReconcilingChildrenSuspenseNodes; // Next, we'll pop back out of the SuspenseNode that we added above and now we'll - // reconcile the fallback, reconciling anything by inserting into the parent SuspenseNode. + // reconcile the fallback, reconciling anything in the context of the parent SuspenseNode. // Since the fallback conceptually blocks the parent. reconcilingParentSuspenseNode = stashedSuspenseParent; previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; try { - updateFlags |= updateVirtualChildrenRecursively( - nextFallbackFiber, - null, - prevFallbackFiber, - traceNearestHostComponentUpdate, - 0, - ); + if (nextFallbackFiber === null) { + unmountRemainingChildren(); + } else { + updateFlags |= updateVirtualChildrenRecursively( + nextFallbackFiber, + null, + prevFallbackFiber, + traceNearestHostComponentUpdate, + 0, + ); + } } finally { reconcilingParentSuspenseNode = fallbackStashedSuspenseParent; previouslyReconciledSiblingSuspenseNode = @@ -4763,7 +4767,8 @@ export function attach( remainingReconcilingChildrenSuspenseNodes = fallbackStashedSuspenseRemaining; } - } else if (nextFiber.memoizedState === null) { + } + if (nextFiber.memoizedState === null) { // Measure this Suspense node in case it changed. We don't update the rect while // we're inside a disconnected subtree nor if we are the Suspense boundary that // is suspended. This lets us keep the rectangle of the displayed content while