From cf6e502ed23ab3357e58965789324ddfa0e12821 Mon Sep 17 00:00:00 2001 From: Sophie Alpert Date: Sat, 9 Aug 2025 08:02:22 -0700 Subject: [PATCH 01/24] Hot reloading: Avoid stack overflow on wide trees (#34145) Every sibling added to the stack here. Not sure this needs to be recursive at all but certainly for siblings this can just be a loop. --- .../src/ReactFiberHotReloading.js | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.js b/packages/react-reconciler/src/ReactFiberHotReloading.js index 3bf8e98d86..984f832359 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.js @@ -261,74 +261,74 @@ function scheduleFibersWithFamiliesRecursively( staleFamilies: Set, ): void { if (__DEV__) { - const {alternate, child, sibling, tag, type} = fiber; + do { + const {alternate, child, sibling, tag, type} = fiber; - let candidateType = null; - switch (tag) { - case FunctionComponent: - case SimpleMemoComponent: - case ClassComponent: - candidateType = type; - break; - case ForwardRef: - candidateType = type.render; - break; - default: - break; - } + let candidateType = null; + switch (tag) { + case FunctionComponent: + case SimpleMemoComponent: + case ClassComponent: + candidateType = type; + break; + case ForwardRef: + candidateType = type.render; + break; + default: + break; + } - if (resolveFamily === null) { - throw new Error('Expected resolveFamily to be set during hot reload.'); - } + if (resolveFamily === null) { + throw new Error('Expected resolveFamily to be set during hot reload.'); + } - let needsRender = false; - let needsRemount = false; - if (candidateType !== null) { - const family = resolveFamily(candidateType); - if (family !== undefined) { - if (staleFamilies.has(family)) { - needsRemount = true; - } else if (updatedFamilies.has(family)) { - if (tag === ClassComponent) { + let needsRender = false; + let needsRemount = false; + if (candidateType !== null) { + const family = resolveFamily(candidateType); + if (family !== undefined) { + if (staleFamilies.has(family)) { needsRemount = true; - } else { - needsRender = true; + } else if (updatedFamilies.has(family)) { + if (tag === ClassComponent) { + needsRemount = true; + } else { + needsRender = true; + } } } } - } - if (failedBoundaries !== null) { - if ( - failedBoundaries.has(fiber) || - // $FlowFixMe[incompatible-use] found when upgrading Flow - (alternate !== null && failedBoundaries.has(alternate)) - ) { - needsRemount = true; + if (failedBoundaries !== null) { + if ( + failedBoundaries.has(fiber) || + // $FlowFixMe[incompatible-use] found when upgrading Flow + (alternate !== null && failedBoundaries.has(alternate)) + ) { + needsRemount = true; + } } - } - if (needsRemount) { - fiber._debugNeedsRemount = true; - } - if (needsRemount || needsRender) { - const root = enqueueConcurrentRenderForLane(fiber, SyncLane); - if (root !== null) { - scheduleUpdateOnFiber(root, fiber, SyncLane); + if (needsRemount) { + fiber._debugNeedsRemount = true; } - } - if (child !== null && !needsRemount) { - scheduleFibersWithFamiliesRecursively( - child, - updatedFamilies, - staleFamilies, - ); - } - if (sibling !== null) { - scheduleFibersWithFamiliesRecursively( - sibling, - updatedFamilies, - staleFamilies, - ); - } + if (needsRemount || needsRender) { + const root = enqueueConcurrentRenderForLane(fiber, SyncLane); + if (root !== null) { + scheduleUpdateOnFiber(root, fiber, SyncLane); + } + } + if (child !== null && !needsRemount) { + scheduleFibersWithFamiliesRecursively( + child, + updatedFamilies, + staleFamilies, + ); + } + + if (sibling === null) { + break; + } + fiber = sibling; + } while (true); } } From 98286cf8e36d67fdef2a225c212cc0f6e62b920e Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Sun, 10 Aug 2025 10:12:20 +0200 Subject: [PATCH 02/24] [DevTools] Send suspense nodes to frontend store (#34070) --- .../src/backend/fiber/renderer.js | 384 +++++++++++++----- .../react-devtools-shared/src/constants.js | 3 + .../src/devtools/store.js | 227 ++++++++++- .../src/devtools/views/DevTools.js | 104 ++--- .../views/Profiler/CommitTreeBuilder.js | 47 +++ .../devtools/views/SuspenseTab/SuspenseTab.js | 5 +- .../views/SuspenseTab/SuspenseTreeContext.js | 111 +++++ .../views/SuspenseTab/SuspenseTreeList.js | 90 ++++ .../src/frontend/types.js | 7 + packages/react-devtools-shared/src/utils.js | 44 +- .../src/app/SuspenseTree/index.js | 150 ++++++- 11 files changed, 1001 insertions(+), 171 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js create mode 100644 packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeList.js diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index b26da3530b..236b31a3d9 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -78,6 +78,9 @@ import { TREE_OPERATION_SET_SUBTREE_MODE, TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, + SUSPENSE_TREE_OPERATION_ADD, + SUSPENSE_TREE_OPERATION_REMOVE, + SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, } from '../../constants'; import {inspectHooksOfFiber} from 'react-debug-tools'; import { @@ -824,8 +827,12 @@ const rootToFiberInstanceMap: Map = new Map(); // Map of id to FiberInstance or VirtualInstance. // This Map is used to e.g. get the display name for a Fiber or schedule an update, // operations that should be the same whether the current and work-in-progress Fiber is used. -const idToDevToolsInstanceMap: Map = - new Map(); +const idToDevToolsInstanceMap: Map< + FiberInstance['id'] | VirtualInstance['id'], + FiberInstance | VirtualInstance, +> = new Map(); + +const idToSuspenseNodeMap: Map = new Map(); // Map of canonical HostInstances to the nearest parent DevToolsInstance. const publicInstanceToDevToolsInstanceMap: Map = @@ -1960,11 +1967,12 @@ export function attach( }; const pendingOperations: OperationsArray = []; - const pendingRealUnmountedIDs: Array = []; + const pendingRealUnmountedIDs: Array = []; + const pendingRealUnmountedSuspenseIDs: Array = []; let pendingOperationsQueue: Array | null = []; const pendingStringTable: Map = new Map(); let pendingStringTableLength: number = 0; - let pendingUnmountedRootID: number | null = null; + let pendingUnmountedRootID: FiberInstance['id'] | null = null; function pushOperation(op: number): void { if (__DEV__) { @@ -1991,6 +1999,7 @@ export function attach( return ( pendingOperations.length === 0 && pendingRealUnmountedIDs.length === 0 && + pendingRealUnmountedSuspenseIDs.length === 0 && pendingUnmountedRootID === null ); } @@ -2056,6 +2065,7 @@ export function attach( const numUnmountIDs = pendingRealUnmountedIDs.length + (pendingUnmountedRootID === null ? 0 : 1); + const numUnmountSuspenseIDs = pendingRealUnmountedSuspenseIDs.length; const operations = new Array( // Identify which renderer this update is coming from. @@ -2064,6 +2074,9 @@ export function attach( 1 + // [stringTableLength] // Then goes the actual string table. pendingStringTableLength + + // All unmounts of Suspense boundaries are batched in a single message. + // [TREE_OPERATION_REMOVE_SUSPENSE, removedSuspenseIDLength, ...ids] + (numUnmountSuspenseIDs > 0 ? 2 + numUnmountSuspenseIDs : 0) + // All unmounts are batched in a single message. // [TREE_OPERATION_REMOVE, removedIDLength, ...ids] (numUnmountIDs > 0 ? 2 + numUnmountIDs : 0) + @@ -2101,6 +2114,19 @@ export function attach( i += length; }); + if (numUnmountSuspenseIDs > 0) { + // All unmounts of Suspense boundaries are batched in a single message. + operations[i++] = SUSPENSE_TREE_OPERATION_REMOVE; + // The first number is how many unmounted IDs we're gonna send. + operations[i++] = numUnmountSuspenseIDs; + // Fill in the real unmounts in the reverse order. + // They were inserted parents-first by React, but we want children-first. + // So we traverse our array backwards. + for (let j = 0; j < pendingRealUnmountedSuspenseIDs.length; j++) { + operations[i++] = pendingRealUnmountedSuspenseIDs[j]; + } + } + if (numUnmountIDs > 0) { // All unmounts except roots are batched in a single message. operations[i++] = TREE_OPERATION_REMOVE; @@ -2130,6 +2156,7 @@ export function attach( // Reset all of the pending state now that we've told the frontend about it. pendingOperations.length = 0; pendingRealUnmountedIDs.length = 0; + pendingRealUnmountedSuspenseIDs.length = 0; pendingUnmountedRootID = null; pendingStringTable.clear(); pendingStringTableLength = 0; @@ -2467,6 +2494,54 @@ export function attach( recordConsoleLogs(instance, componentLogsEntry); } + function recordSuspenseMount( + suspenseInstance: SuspenseNode, + parentSuspenseInstance: SuspenseNode | null, + ): void { + const fiberInstance = suspenseInstance.instance; + if (fiberInstance.kind === FILTERED_FIBER_INSTANCE) { + throw new Error('Cannot record a mount for a filtered Fiber instance.'); + } + const fiberID = fiberInstance.id; + + let unfilteredParent = parentSuspenseInstance; + while ( + unfilteredParent !== null && + unfilteredParent.instance.kind === FILTERED_FIBER_INSTANCE + ) { + unfilteredParent = unfilteredParent.parent; + } + const unfilteredParentInstance = + unfilteredParent !== null ? unfilteredParent.instance : null; + if ( + unfilteredParentInstance !== null && + unfilteredParentInstance.kind === FILTERED_FIBER_INSTANCE + ) { + throw new Error( + 'Should not have a filtered instance at this point. This is a bug.', + ); + } + const parentID = + unfilteredParentInstance === null ? 0 : unfilteredParentInstance.id; + + const fiber = fiberInstance.data; + const props = fiber.memoizedProps; + // TODO: Compute a fallback name based on Owner, key etc. + const name = props === null ? null : props.name || null; + const nameStringID = getStringID(name); + + if (__DEBUG__) { + console.log('recordSuspenseMount()', suspenseInstance); + } + + idToSuspenseNodeMap.set(fiberID, suspenseInstance); + + pushOperation(SUSPENSE_TREE_OPERATION_ADD); + pushOperation(fiberID); + pushOperation(parentID); + pushOperation(nameStringID); + } + function recordUnmount(fiberInstance: FiberInstance): void { if (__DEBUG__) { debug('recordUnmount()', fiberInstance, reconcilingParent); @@ -2474,6 +2549,11 @@ export function attach( recordDisconnect(fiberInstance); + const suspenseNode = fiberInstance.suspenseNode; + if (suspenseNode !== null) { + recordSuspenseUnmount(suspenseNode); + } + idToDevToolsInstanceMap.delete(fiberInstance.id); untrackFiber(fiberInstance, fiberInstance.data); @@ -2511,6 +2591,30 @@ export function attach( // TODO: Notify the front end of the change. } + function recordSuspenseUnmount(suspenseInstance: SuspenseNode): void { + if (__DEBUG__) { + console.log( + 'recordSuspenseUnmount()', + suspenseInstance, + reconcilingParentSuspenseNode, + ); + } + + const devtoolsInstance = suspenseInstance.instance; + if (devtoolsInstance.kind !== FIBER_INSTANCE) { + throw new Error("Can't unmount a filtered SuspenseNode. This is a bug."); + } + const fiberInstance = devtoolsInstance; + const id = fiberInstance.id; + + // To maintain child-first ordering, + // we'll push it into one of these queues, + // and later arrange them in the correct order. + pendingRealUnmountedSuspenseIDs.push(id); + + idToSuspenseNodeMap.delete(id); + } + // Running state of the remaining children from the previous version of this parent that // we haven't yet added back. This should be reset anytime we change parent. // Any remaining ones at the end will be deleted. @@ -3181,6 +3285,7 @@ export function attach( // inserted the new children but since we know this is a FiberInstance we'll // just use the Fiber anyway. newSuspenseNode.rects = measureInstance(newInstance); + recordSuspenseMount(newSuspenseNode, reconcilingParentSuspenseNode); } insertChild(newInstance); if (__DEBUG__) { @@ -3609,6 +3714,56 @@ export function attach( } } + function addUnfilteredSuspenseChildrenIDs( + parentInstance: SuspenseNode, + nextChildren: Array, + ): void { + let child: null | SuspenseNode = parentInstance.firstChild; + while (child !== null) { + if (child.instance.kind === FILTERED_FIBER_INSTANCE) { + addUnfilteredSuspenseChildrenIDs(child, nextChildren); + } else { + nextChildren.push(child.instance.id); + } + child = child.nextSibling; + } + } + + function recordResetSuspenseChildren(parentInstance: SuspenseNode) { + if (__DEBUG__) { + if (parentInstance.firstChild !== null) { + console.log( + 'recordResetSuspenseChildren()', + parentInstance.firstChild, + parentInstance, + ); + } + } + // The frontend only really cares about the name, and children. + // The first two don't really change, so we are only concerned with the order of children here. + // This is trickier than a simple comparison though, since certain types of fibers are filtered. + const nextChildren: Array = []; + + addUnfilteredSuspenseChildrenIDs(parentInstance, nextChildren); + + const numChildren = nextChildren.length; + if (numChildren < 2) { + // No need to reorder. + return; + } + pushOperation(SUSPENSE_TREE_OPERATION_REORDER_CHILDREN); + // $FlowFixMe[incompatible-call] TODO: Allow filtering SuspenseNode + pushOperation(parentInstance.instance.id); + pushOperation(numChildren); + for (let i = 0; i < nextChildren.length; i++) { + pushOperation(nextChildren[i]); + } + } + + const NoUpdate = /* */ 0b00; + const ShouldResetChildren = /* */ 0b01; + const ShouldResetSuspenseChildren = /* */ 0b10; + function updateVirtualInstanceRecursively( virtualInstance: VirtualInstance, nextFirstChild: Fiber, @@ -3616,7 +3771,7 @@ export function attach( prevFirstChild: null | Fiber, traceNearestHostComponentUpdate: boolean, virtualLevel: number, // the nth level of virtual instances - ): void { + ): number { const stashedParent = reconcilingParent; const stashedPrevious = previouslyReconciledSibling; const stashedRemaining = remainingReconcilingChildren; @@ -3630,16 +3785,16 @@ export function attach( virtualInstance.firstChild = null; virtualInstance.suspendedBy = null; try { - if ( - updateVirtualChildrenRecursively( - nextFirstChild, - nextLastChild, - prevFirstChild, - traceNearestHostComponentUpdate, - virtualLevel + 1, - ) - ) { + let updateFlags = updateVirtualChildrenRecursively( + nextFirstChild, + nextLastChild, + prevFirstChild, + traceNearestHostComponentUpdate, + virtualLevel + 1, + ); + if ((updateFlags & ShouldResetChildren) !== NoUpdate) { recordResetChildren(virtualInstance); + updateFlags &= ~ShouldResetChildren; } removePreviousSuspendedBy(virtualInstance, previousSuspendedBy); // Update the errors/warnings count. If this Instance has switched to a different @@ -3652,6 +3807,8 @@ export function attach( recordConsoleLogs(virtualInstance, componentLogsEntry); // Must be called after all children have been appended. recordVirtualProfilingDurations(virtualInstance); + + return updateFlags; } finally { unmountRemainingChildren(); reconcilingParent = stashedParent; @@ -3666,8 +3823,8 @@ export function attach( prevFirstChild: null | Fiber, traceNearestHostComponentUpdate: boolean, virtualLevel: number, // the nth level of virtual instances - ): boolean { - let shouldResetChildren = false; + ): number { + let updateFlags = NoUpdate; // If the first child is different, we need to traverse them. // Each next child will be either a new child (mount) or an alternate (update). let nextChild: null | Fiber = nextFirstChild; @@ -3727,8 +3884,10 @@ export function attach( traceNearestHostComponentUpdate, virtualLevel, ); + updateFlags |= + ShouldResetChildren | ShouldResetSuspenseChildren; } else { - updateVirtualInstanceRecursively( + updateFlags |= updateVirtualInstanceRecursively( previousVirtualInstance, previousVirtualInstanceNextFirstFiber, nextChild, @@ -3779,7 +3938,7 @@ export function attach( insertChild(newVirtualInstance); previousVirtualInstance = newVirtualInstance; previousVirtualInstanceWasMount = true; - shouldResetChildren = true; + updateFlags |= ShouldResetChildren; } // Existing children might be reparented into this new virtual instance. // TODO: This will cause the front end to error which needs to be fixed. @@ -3806,8 +3965,9 @@ export function attach( traceNearestHostComponentUpdate, virtualLevel, ); + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } else { - updateVirtualInstanceRecursively( + updateFlags |= updateVirtualInstanceRecursively( previousVirtualInstance, previousVirtualInstanceNextFirstFiber, nextChild, @@ -3857,44 +4017,36 @@ export function attach( // They are always different referentially, but if the instances line up // conceptually we'll want to know that. if (prevChild !== prevChildAtSameIndex) { - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } moveChild(fiberInstance, previousSiblingOfExistingInstance); - if ( - updateFiberRecursively( - fiberInstance, - nextChild, - (prevChild: any), - traceNearestHostComponentUpdate, - ) - ) { - // If a nested tree child order changed but it can't handle its own - // child order invalidation (e.g. because it's filtered out like host nodes), - // propagate the need to reset child order upwards to this Fiber. - shouldResetChildren = true; - } + // If a nested tree child order changed but it can't handle its own + // child order invalidation (e.g. because it's filtered out like host nodes), + // propagate the need to reset child order upwards to this Fiber. + updateFlags |= updateFiberRecursively( + fiberInstance, + nextChild, + (prevChild: any), + traceNearestHostComponentUpdate, + ); } else if (prevChild !== null && shouldFilterFiber(nextChild)) { // The filtered instance could've reordered. if (prevChild !== prevChildAtSameIndex) { - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } // If this Fiber should be filtered, we need to still update its children. // This relies on an alternate since we don't have an Instance with the previous // child on it. Ideally, the reconciliation wouldn't need previous Fibers that // are filtered from the tree. - if ( - updateFiberRecursively( - null, - nextChild, - prevChild, - traceNearestHostComponentUpdate, - ) - ) { - shouldResetChildren = true; - } + updateFlags |= updateFiberRecursively( + null, + nextChild, + prevChild, + traceNearestHostComponentUpdate, + ); } else { // It's possible for a FiberInstance to be reparented when virtual parents // get their sequence split or change structure with the same render result. @@ -3906,14 +4058,17 @@ export function attach( mountFiberRecursively(nextChild, traceNearestHostComponentUpdate); // Need to mark the parent set to remount the new instance. - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } } // Try the next child. nextChild = nextChild.sibling; // Advance the pointer in the previous list so that we can // keep comparing if they line up. - if (!shouldResetChildren && prevChildAtSameIndex !== null) { + if ( + (updateFlags & ShouldResetChildren) === NoUpdate && + prevChildAtSameIndex !== null + ) { prevChildAtSameIndex = prevChildAtSameIndex.sibling; } } @@ -3926,8 +4081,9 @@ export function attach( traceNearestHostComponentUpdate, virtualLevel, ); + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } else { - updateVirtualInstanceRecursively( + updateFlags |= updateVirtualInstanceRecursively( previousVirtualInstance, previousVirtualInstanceNextFirstFiber, null, @@ -3939,9 +4095,9 @@ export function attach( } // If we have no more children, but used to, they don't line up. if (prevChildAtSameIndex !== null) { - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } - return shouldResetChildren; + return updateFlags; } // Returns whether closest unfiltered fiber parent needs to reset its child list. @@ -3949,9 +4105,9 @@ export function attach( nextFirstChild: null | Fiber, prevFirstChild: null | Fiber, traceNearestHostComponentUpdate: boolean, - ): boolean { + ): number { if (nextFirstChild === null) { - return prevFirstChild !== null; + return prevFirstChild !== null ? ShouldResetChildren : NoUpdate; } return updateVirtualChildrenRecursively( nextFirstChild, @@ -3968,7 +4124,7 @@ export function attach( nextFiber: Fiber, prevFiber: Fiber, traceNearestHostComponentUpdate: boolean, - ): boolean { + ): number { if (__DEBUG__) { if (fiberInstance !== null) { debug('updateFiberRecursively()', fiberInstance, reconcilingParent); @@ -4067,7 +4223,7 @@ export function attach( aquireHostInstance(nearestInstance, nextFiber.stateNode); } - let shouldResetChildren = false; + let updateFlags = NoUpdate; // The behavior of timed-out legacy Suspense trees is unique. Without the Offscreen wrapper. // Rather than unmount the timed out content (and possibly lose important state), @@ -4110,20 +4266,18 @@ export function attach( traceNearestHostComponentUpdate, ); - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } - if ( - nextFallbackChildSet != null && - prevFallbackChildSet != null && - updateChildrenRecursively( - nextFallbackChildSet, - prevFallbackChildSet, - traceNearestHostComponentUpdate, - ) - ) { - shouldResetChildren = true; - } + const childrenUpdateFlags = + nextFallbackChildSet != null && prevFallbackChildSet != null + ? updateChildrenRecursively( + nextFallbackChildSet, + prevFallbackChildSet, + traceNearestHostComponentUpdate, + ) + : NoUpdate; + updateFlags |= childrenUpdateFlags; } else if (prevDidTimeout && !nextDidTimeOut) { // Fallback -> Primary: // 1. Unmount fallback set @@ -4135,8 +4289,8 @@ export function attach( nextPrimaryChildSet, traceNearestHostComponentUpdate, ); + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } - shouldResetChildren = true; } else if (!prevDidTimeout && nextDidTimeOut) { // Primary -> Fallback: // 1. Hide primary set @@ -4152,7 +4306,7 @@ export function attach( nextFallbackChildSet, traceNearestHostComponentUpdate, ); - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } } else if (nextIsHidden) { if (!prevWasHidden) { @@ -4165,7 +4319,11 @@ export function attach( const stashedDisconnected = isInDisconnectedSubtree; isInDisconnectedSubtree = true; try { - updateChildrenRecursively(nextFiber.child, prevFiber.child, false); + updateFlags |= updateChildrenRecursively( + nextFiber.child, + prevFiber.child, + false, + ); } finally { isInDisconnectedSubtree = stashedDisconnected; } @@ -4177,7 +4335,11 @@ export function attach( isInDisconnectedSubtree = true; try { if (nextFiber.child !== null) { - updateChildrenRecursively(nextFiber.child, prevFiber.child, false); + updateFlags |= updateChildrenRecursively( + nextFiber.child, + prevFiber.child, + false, + ); } // Ensure we unmount any remaining children inside the isInDisconnectedSubtree flag // since they should not trigger real deletions. @@ -4189,7 +4351,7 @@ export function attach( if (fiberInstance !== null && !isInDisconnectedSubtree) { reconnectChildrenRecursively(fiberInstance); // Children may have reordered while they were hidden. - shouldResetChildren = true; + updateFlags |= ShouldResetChildren | ShouldResetSuspenseChildren; } } else if ( nextFiber.tag === SuspenseComponent && @@ -4209,17 +4371,13 @@ export function attach( const nextFallbackFiber = nextContentFiber.sibling; // First update only the Offscreen boundary. I.e. the main content. - if ( - updateVirtualChildrenRecursively( - nextContentFiber, - nextFallbackFiber, - prevContentFiber, - traceNearestHostComponentUpdate, - 0, - ) - ) { - shouldResetChildren = true; - } + updateFlags |= updateVirtualChildrenRecursively( + nextContentFiber, + nextFallbackFiber, + prevContentFiber, + traceNearestHostComponentUpdate, + 0, + ); // 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. @@ -4229,17 +4387,13 @@ export function attach( remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; shouldPopSuspenseNode = false; if (nextFallbackFiber !== null) { - if ( - updateVirtualChildrenRecursively( - nextFallbackFiber, - null, - prevFallbackFiber, - traceNearestHostComponentUpdate, - 0, - ) - ) { - shouldResetChildren = true; - } + updateFlags |= updateVirtualChildrenRecursively( + nextFallbackFiber, + null, + prevFallbackFiber, + traceNearestHostComponentUpdate, + 0, + ); } else if ( nextFiber.memoizedState === null && fiberInstance.suspenseNode !== null @@ -4262,15 +4416,11 @@ export function attach( // Common case: Primary -> Primary. // This is the same code path as for non-Suspense fibers. if (nextFiber.child !== prevFiber.child) { - if ( - updateChildrenRecursively( - nextFiber.child, - prevFiber.child, - traceNearestHostComponentUpdate, - ) - ) { - shouldResetChildren = true; - } + updateFlags |= updateChildrenRecursively( + nextFiber.child, + prevFiber.child, + traceNearestHostComponentUpdate, + ); } else { // Children are unchanged. if (fiberInstance !== null) { @@ -4293,15 +4443,19 @@ export function attach( } } } else { + const childrenUpdateFlags = updateChildrenRecursively( + nextFiber.child, + prevFiber.child, + false, + ); // If this fiber is filtered there might be changes to this set elsewhere so we have // to visit each child to place it back in the set. We let the child bail out instead. - if ( - updateChildrenRecursively(nextFiber.child, prevFiber.child, false) - ) { + if ((childrenUpdateFlags & ShouldResetChildren) !== NoUpdate) { throw new Error( 'The children should not have changed if we pass in the same set.', ); } + updateFlags |= childrenUpdateFlags; } } } @@ -4330,21 +4484,35 @@ export function attach( } } } - if (shouldResetChildren) { + + if ((updateFlags & ShouldResetChildren) !== NoUpdate) { // We need to crawl the subtree for closest non-filtered Fibers // so that we can display them in a flat children set. if (fiberInstance !== null && fiberInstance.kind === FIBER_INSTANCE) { recordResetChildren(fiberInstance); + // We've handled the child order change for this Fiber. // Since it's included, there's no need to invalidate parent child order. - return false; + updateFlags &= ~ShouldResetChildren; } else { // Let the closest unfiltered parent Fiber reset its child order instead. - return true; } } else { - return false; } + + if ((updateFlags & ShouldResetSuspenseChildren) !== NoUpdate) { + if (fiberInstance !== null && fiberInstance.kind === FIBER_INSTANCE) { + const suspenseNode = fiberInstance.suspenseNode; + if (suspenseNode !== null) { + recordResetSuspenseChildren(suspenseNode); + updateFlags &= ~ShouldResetSuspenseChildren; + } + } else { + // Let the closest unfiltered parent Fiber reset its child order instead. + } + } + + return updateFlags; } finally { if (fiberInstance !== null) { unmountRemainingChildren(); diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index fa32ead1e9..391eea6b23 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -24,6 +24,9 @@ export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; export const TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS = 5; export const TREE_OPERATION_REMOVE_ROOT = 6; export const TREE_OPERATION_SET_SUBTREE_MODE = 7; +export const SUSPENSE_TREE_OPERATION_ADD = 8; +export const SUSPENSE_TREE_OPERATION_REMOVE = 9; +export const SUSPENSE_TREE_OPERATION_REORDER_CHILDREN = 10; export const PROFILING_FLAG_BASIC_SUPPORT = 0b01; export const PROFILING_FLAG_TIMELINE_SUPPORT = 0b10; diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 3035c0ae4a..622c9a4754 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -20,6 +20,9 @@ import { TREE_OPERATION_SET_SUBTREE_MODE, TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, + SUSPENSE_TREE_OPERATION_ADD, + SUSPENSE_TREE_OPERATION_REMOVE, + SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, } from '../constants'; import {ElementTypeRoot} from '../frontend/types'; import { @@ -44,6 +47,7 @@ import type { Element, ComponentFilter, ElementType, + SuspenseNode, } from 'react-devtools-shared/src/frontend/types'; import type { FrontendBridge, @@ -100,11 +104,12 @@ export default class Store extends EventEmitter<{ hookSettings: [$ReadOnly], hostInstanceSelected: [Element['id']], settingsUpdated: [$ReadOnly], - mutated: [[Array, Map]], + mutated: [[Array, Map]], recordChangeDescriptions: [], roots: [], rootSupportsBasicProfiling: [], rootSupportsTimelineProfiling: [], + suspenseTreeMutated: [], supportsNativeStyleEditor: [], supportsReloadAndProfile: [], unsupportedBridgeProtocolDetected: [], @@ -127,8 +132,10 @@ export default class Store extends EventEmitter<{ _componentFilters: Array; // Map of ID to number of recorded error and warning message IDs. - _errorsAndWarnings: Map = - new Map(); + _errorsAndWarnings: Map< + Element['id'], + {errorCount: number, warningCount: number}, + > = new Map(); // At least one of the injected renderers contains (DEV only) owner metadata. _hasOwnerMetadata: boolean = false; @@ -136,7 +143,9 @@ export default class Store extends EventEmitter<{ // Map of ID to (mutable) Element. // Elements are mutated to avoid excessive cloning during tree updates. // The InspectedElement Suspense cache also relies on this mutability for its WeakMap usage. - _idToElement: Map = new Map(); + _idToElement: Map = new Map(); + + _idToSuspense: Map = new Map(); // Should the React Native style editor panel be shown? _isNativeStyleEditorSupported: boolean = false; @@ -149,7 +158,7 @@ export default class Store extends EventEmitter<{ // Map of element (id) to the set of elements (ids) it owns. // This map enables getOwnersListForElement() to avoid traversing the entire tree. - _ownersMap: Map> = new Map(); + _ownersMap: Map> = new Map(); _profilerStore: ProfilerStore; @@ -158,15 +167,16 @@ export default class Store extends EventEmitter<{ // Incremented each time the store is mutated. // This enables a passive effect to detect a mutation between render and commit phase. _revision: number = 0; + _revisionSuspense: number = 0; // This Array must be treated as immutable! // Passive effects will check it for changes between render and mount. - _roots: $ReadOnlyArray = []; + _roots: $ReadOnlyArray = []; - _rootIDToCapabilities: Map = new Map(); + _rootIDToCapabilities: Map = new Map(); // Renderer ID is needed to support inspection fiber props, state, and hooks. - _rootIDToRendererID: Map = new Map(); + _rootIDToRendererID: Map = new Map(); // These options may be initially set by a configuration option when constructing the Store. _supportsInspectMatchingDOMElement: boolean = false; @@ -439,6 +449,9 @@ export default class Store extends EventEmitter<{ get revision(): number { return this._revision; } + get revisionSuspense(): number { + return this._revisionSuspense; + } get rootIDToRendererID(): Map { return this._rootIDToRendererID; @@ -595,6 +608,16 @@ export default class Store extends EventEmitter<{ return element; } + getSuspenseByID(id: SuspenseNode['id']): SuspenseNode | null { + const suspense = this._idToSuspense.get(id); + if (suspense === undefined) { + console.warn(`No suspense found with id "${id}"`); + return null; + } + + return suspense; + } + // Returns a tuple of [id, index] getElementsWithErrorsAndWarnings(): ErrorAndWarningTuples { if (!this._shouldShowWarningsAndErrors) { @@ -989,6 +1012,7 @@ export default class Store extends EventEmitter<{ let haveRootsChanged = false; let haveErrorsOrWarningsChanged = false; + let hasSuspenseTreeChanged = false; // The first two values are always rendererID and rootID const rendererID = operations[0]; @@ -1369,7 +1393,7 @@ export default class Store extends EventEmitter<{ // The profiler UI uses them lazily in order to generate the tree. i += 3; break; - case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: + case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: { const id = operations[i + 1]; const errorCount = operations[i + 2]; const warningCount = operations[i + 3]; @@ -1383,6 +1407,184 @@ export default class Store extends EventEmitter<{ } haveErrorsOrWarningsChanged = true; break; + } + case SUSPENSE_TREE_OPERATION_ADD: { + const id = operations[i + 1]; + const parentID = operations[i + 2]; + const nameStringID = operations[i + 3]; + let name = stringTable[nameStringID]; + + if (this._idToSuspense.has(id)) { + this._throwAndEmitError( + Error( + `Cannot add suspense node "${id}" because a suspense node with that id is already in the Store.`, + ), + ); + } + + const element = this._idToElement.get(id); + if (element === undefined) { + this._throwAndEmitError( + Error( + `Cannot add suspense node "${id}" because no matching element was found in the Store.`, + ), + ); + } else { + if (name === null) { + // The boundary isn't explicitly named. + // Pick a sensible default. + // TODO: Use key + const owner = this._idToElement.get(element.ownerID); + if (owner !== undefined) { + // TODO: This is clowny + name = `${owner.displayName || 'Unknown'}>?`; + } + } + } + + if (__DEBUG__) { + debug('Suspense Add', `node ${id} as child of ${parentID}`); + } + + if (parentID !== 0) { + const parentSuspense = this._idToSuspense.get(parentID); + if (parentSuspense === undefined) { + this._throwAndEmitError( + Error( + `Cannot add suspense child "${id}" to parent suspense "${parentID}" because parent suspense node was not found in the Store.`, + ), + ); + + break; + } + + parentSuspense.children.push(id); + } + + if (name === null) { + name = 'Unknown'; + } + + this._idToSuspense.set(id, { + id, + parentID, + children: [], + name, + }); + + i += 4; + + hasSuspenseTreeChanged = true; + break; + } + case SUSPENSE_TREE_OPERATION_REMOVE: { + const removeLength = operations[i + 1]; + i += 2; + + for (let removeIndex = 0; removeIndex < removeLength; removeIndex++) { + const id = operations[i]; + const suspense = this._idToSuspense.get(id); + + if (suspense === undefined) { + this._throwAndEmitError( + Error( + `Cannot remove suspense node "${id}" because no matching node was found in the Store.`, + ), + ); + + break; + } + + i += 1; + + const {children, parentID} = suspense; + if (children.length > 0) { + this._throwAndEmitError( + Error(`Suspense node "${id}" was removed before its children.`), + ); + } + + this._idToSuspense.delete(id); + + let parentSuspense: ?SuspenseNode = null; + if (parentID === 0) { + if (__DEBUG__) { + debug('Suspense remove', `node ${id} root`); + } + } else { + if (__DEBUG__) { + debug('Suspense Remove', `node ${id} from parent ${parentID}`); + } + + parentSuspense = this._idToSuspense.get(parentID); + if (parentSuspense === undefined) { + this._throwAndEmitError( + Error( + `Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, + ), + ); + + break; + } + + const index = parentSuspense.children.indexOf(id); + parentSuspense.children.splice(index, 1); + } + } + + hasSuspenseTreeChanged = true; + break; + } + case SUSPENSE_TREE_OPERATION_REORDER_CHILDREN: { + const id = operations[i + 1]; + const numChildren = operations[i + 2]; + i += 3; + + const suspense = this._idToSuspense.get(id); + if (suspense === undefined) { + this._throwAndEmitError( + Error( + `Cannot reorder children for suspense node "${id}" because no matching node was found in the Store.`, + ), + ); + + break; + } + + const children = suspense.children; + if (children.length !== numChildren) { + this._throwAndEmitError( + Error( + `Suspense children cannot be added or removed during a reorder operation.`, + ), + ); + } + + for (let j = 0; j < numChildren; j++) { + const childID = operations[i + j]; + children[j] = childID; + if (__DEV__) { + // This check is more expensive so it's gated by __DEV__. + const childSuspense = this._idToSuspense.get(childID); + if (childSuspense == null || childSuspense.parentID !== id) { + console.error( + `Suspense children cannot be added or removed during a reorder operation.`, + ); + } + } + } + i += numChildren; + + if (__DEBUG__) { + debug( + 'Re-order', + `Suspense node ${id} children ${children.join(',')}`, + ); + } + + hasSuspenseTreeChanged = true; + break; + } default: this._throwAndEmitError( new UnsupportedBridgeOperationError( @@ -1393,6 +1595,9 @@ export default class Store extends EventEmitter<{ } this._revision++; + if (hasSuspenseTreeChanged) { + this._revisionSuspense++; + } // Any time the tree changes (e.g. elements added, removed, or reordered) cached indices may be invalid. this._cachedErrorAndWarningTuples = null; @@ -1451,6 +1656,10 @@ export default class Store extends EventEmitter<{ } } + if (hasSuspenseTreeChanged) { + this.emit('suspenseTreeMutated'); + } + if (__DEBUG__) { console.log(printStore(this, true)); console.groupEnd(); diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index fa02555e4c..91a17dcad2 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -33,6 +33,7 @@ import FetchFileWithCachingContext from './Components/FetchFileWithCachingContex import {InspectedElementContextController} from './Components/InspectedElementContext'; import HookNamesModuleLoaderContext from 'react-devtools-shared/src/devtools/views/Components/HookNamesModuleLoaderContext'; import {ProfilerContextController} from './Profiler/ProfilerContext'; +import {SuspenseTreeContextController} from './SuspenseTab/SuspenseTreeContext'; import {TimelineContextController} from 'react-devtools-timeline/src/TimelineContext'; import {ModalDialogContextController} from './ModalDialog'; import ReactLogo from './ReactLogo'; @@ -319,58 +320,65 @@ export default function DevTools({ - -
- {showTabBar && ( -
- - - {process.env.DEVTOOLS_VERSION} - -
- + +
+ {showTabBar && ( +
+ + + {process.env.DEVTOOLS_VERSION} + +
+ +
+ )} + + + - )} - - - -
- {editorPortalContainer ? ( - - ) : null} - + ) : null} + + diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js index 75c9b8a6d9..dfa515fffa 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -16,6 +16,9 @@ import { TREE_OPERATION_SET_SUBTREE_MODE, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, + SUSPENSE_TREE_OPERATION_ADD, + SUSPENSE_TREE_OPERATION_REMOVE, + SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, } from 'react-devtools-shared/src/constants'; import { parseElementDisplayNameFromBackend, @@ -366,6 +369,50 @@ function updateTree( break; } + case SUSPENSE_TREE_OPERATION_ADD: { + const fiberID = operations[i + 1]; + const parentID = operations[i + 2]; + const nameStringID = operations[i + 3]; + const name = stringTable[nameStringID]; + + i += 4; + + if (__DEBUG__) { + debug( + 'Add suspense', + `node ${fiberID} (${String(name)}) under ${parentID}`, + ); + } + break; + } + + case SUSPENSE_TREE_OPERATION_REMOVE: { + const removeLength = ((operations[i + 1]: any): number); + i += 2 + removeLength; + + break; + } + + case SUSPENSE_TREE_OPERATION_REORDER_CHILDREN: { + const suspenseID = ((operations[i + 1]: any): number); + const numChildren = ((operations[i + 2]: any): number); + const children = ((operations.slice( + i + 3, + i + 3 + numChildren, + ): any): Array); + + i = i + 3 + numChildren; + + if (__DEBUG__) { + debug( + 'Suspense re-order', + `suspense ${suspenseID} children ${children.join(',')}`, + ); + } + + break; + } + default: throw Error(`Unsupported Bridge operation "${operation}"`); } diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js index a920b6dabd..d113fd3901 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js @@ -19,6 +19,7 @@ import InspectedElementErrorBoundary from '../Components/InspectedElementErrorBo import InspectedElement from '../Components/InspectedElement'; import portaledContent from '../portaledContent'; import styles from './SuspenseTab.css'; +import SuspenseTreeList from './SuspenseTreeList'; import Button from '../Button'; type Orientation = 'horizontal' | 'vertical'; @@ -43,10 +44,6 @@ type LayoutState = { }; type LayoutDispatch = (action: LayoutAction) => void; -function SuspenseTreeList() { - return
tree list
; -} - function SuspenseTimeline() { return
timeline
; } diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js new file mode 100644 index 0000000000..8441a99497 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js @@ -0,0 +1,111 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +import type {ReactContext} from 'shared/ReactTypes'; + +import * as React from 'react'; +import { + createContext, + startTransition, + useContext, + useEffect, + useMemo, + useReducer, +} from 'react'; +import {StoreContext} from '../context'; + +export type SuspenseTreeState = {}; + +type ACTION_HANDLE_SUSPENSE_TREE_MUTATION = { + type: 'HANDLE_SUSPENSE_TREE_MUTATION', +}; +export type SuspenseTreeAction = ACTION_HANDLE_SUSPENSE_TREE_MUTATION; +export type SuspenseTreeDispatch = (action: SuspenseTreeAction) => void; + +const SuspenseTreeStateContext: ReactContext = + createContext(((null: any): SuspenseTreeState)); +SuspenseTreeStateContext.displayName = 'SuspenseTreeStateContext'; + +const SuspenseTreeDispatcherContext: ReactContext = + createContext(((null: any): SuspenseTreeDispatch)); +SuspenseTreeDispatcherContext.displayName = 'SuspenseTreeDispatcherContext'; + +type Props = { + children: React$Node, +}; + +function SuspenseTreeContextController({children}: Props): React.Node { + const store = useContext(StoreContext); + + const initialRevision = useMemo(() => store.revisionSuspense, [store]); + + // This reducer is created inline because it needs access to the Store. + // The store is mutable, but the Store itself is global and lives for the lifetime of the DevTools, + // so it's okay for the reducer to have an empty dependencies array. + const reducer = useMemo( + () => + ( + state: SuspenseTreeState, + action: SuspenseTreeAction, + ): SuspenseTreeState => { + const {type} = action; + switch (type) { + case 'HANDLE_SUSPENSE_TREE_MUTATION': + return {...state}; + default: + throw new Error(`Unrecognized action "${type}"`); + } + }, + [], + ); + + const [state, dispatch] = useReducer(reducer, {}); + const transitionDispatch = useMemo( + () => (action: SuspenseTreeAction) => + startTransition(() => { + dispatch(action); + }), + [dispatch], + ); + + useEffect(() => { + const handleSuspenseTreeMutated = () => { + transitionDispatch({ + type: 'HANDLE_SUSPENSE_TREE_MUTATION', + }); + }; + + // Since this is a passive effect, the tree may have been mutated before our initial subscription. + if (store.revisionSuspense !== initialRevision) { + // At the moment, we can treat this as a mutation. + // We don't know which Elements were newly added/removed, but that should be okay in this case. + // It would only impact the search state, which is unlikely to exist yet at this point. + transitionDispatch({ + type: 'HANDLE_SUSPENSE_TREE_MUTATION', + }); + } + + store.addListener('suspenseTreeMutated', handleSuspenseTreeMutated); + return () => + store.removeListener('suspenseTreeMutated', handleSuspenseTreeMutated); + }, [dispatch, initialRevision, store]); + + return ( + + + {children} + + + ); +} + +export { + SuspenseTreeDispatcherContext, + SuspenseTreeStateContext, + SuspenseTreeContextController, +}; diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeList.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeList.js new file mode 100644 index 0000000000..43bee6eb12 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeList.js @@ -0,0 +1,90 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +import type {SuspenseNode} from '../../../frontend/types'; +import type Store from '../../store'; + +import * as React from 'react'; +import {useContext} from 'react'; +import {StoreContext} from '../context'; +import {SuspenseTreeStateContext} from './SuspenseTreeContext'; +import {TreeDispatcherContext} from '../Components/TreeContext'; + +function getDocumentOrderSuspenseTreeList(store: Store): Array { + const suspenseTreeList: SuspenseNode[] = []; + for (let i = 0; i < store.roots.length; i++) { + const root = store.getElementByID(store.roots[i]); + if (root === null) { + continue; + } + const suspense = store.getSuspenseByID(root.id); + if (suspense !== null) { + const stack = [suspense]; + while (stack.length > 0) { + const current = stack.pop(); + if (current === undefined) { + continue; + } + suspenseTreeList.push(current); + // Add children in reverse order to maintain document order + for (let j = current.children.length - 1; j >= 0; j--) { + const childSuspense = store.getSuspenseByID(current.children[j]); + if (childSuspense !== null) { + stack.push(childSuspense); + } + } + } + } + } + + return suspenseTreeList; +} + +export default function SuspenseTreeList(_: {}): React$Node { + const store = useContext(StoreContext); + const treeDispatch = useContext(TreeDispatcherContext); + useContext(SuspenseTreeStateContext); + + const suspenseTreeList = getDocumentOrderSuspenseTreeList(store); + + return ( +
+

Suspense Tree List

+
    + {suspenseTreeList.map(suspense => { + const {id, parentID, children, name} = suspense; + return ( +
  • +
    + +
    +
    + Suspense ID: {id} +
    +
    + Parent ID: {parentID} +
    +
    + Children:{' '} + {children.length === 0 ? '∅' : children.join(', ')} +
    +
  • + ); + })} +
+
+ ); +} diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index e4a4c5400b..3fff08877c 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -184,6 +184,13 @@ export type Element = { compiledWithForget: boolean, }; +export type SuspenseNode = { + id: Element['id'], + parentID: SuspenseNode['id'] | 0, + children: Array, + name: string | null, +}; + // Serialized version of ReactIOInfo export type SerializedIOInfo = { name: string, diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 325224844d..ef5e7450ac 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -40,6 +40,9 @@ import { SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, SESSION_STORAGE_RECORD_TIMELINE_KEY, + SUSPENSE_TREE_OPERATION_ADD, + SUSPENSE_TREE_OPERATION_REMOVE, + SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, } from './constants'; import { ComponentFilterElementType, @@ -318,7 +321,7 @@ export function printOperationsArray(operations: Array) { // The profiler UI uses them lazily in order to generate the tree. i += 3; break; - case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: + case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: { const id = operations[i + 1]; const numErrors = operations[i + 2]; const numWarnings = operations[i + 3]; @@ -329,6 +332,45 @@ export function printOperationsArray(operations: Array) { `Node ${id} has ${numErrors} errors and ${numWarnings} warnings`, ); break; + } + case SUSPENSE_TREE_OPERATION_ADD: { + const fiberID = operations[i + 1]; + const parentID = operations[i + 2]; + const nameStringID = operations[i + 3]; + const name = stringTable[nameStringID]; + + i += 4; + + logs.push( + `Add suspense node ${fiberID} (${String(name)}) under ${parentID}`, + ); + break; + } + case SUSPENSE_TREE_OPERATION_REMOVE: { + const removeLength = ((operations[i + 1]: any): number); + i += 2; + + for (let removeIndex = 0; removeIndex < removeLength; removeIndex++) { + const id = ((operations[i]: any): number); + i += 1; + + logs.push(`Remove suspense node ${id}`); + } + + break; + } + case SUSPENSE_TREE_OPERATION_REORDER_CHILDREN: { + const id = ((operations[i + 1]: any): number); + const numChildren = ((operations[i + 2]: any): number); + i += 3; + const children = operations.slice(i, i + numChildren); + i += numChildren; + + logs.push( + `Re-order suspense node ${id} children ${children.join(',')}`, + ); + break; + } default: throw Error(`Unsupported Bridge operation "${operation}"`); } diff --git a/packages/react-devtools-shell/src/app/SuspenseTree/index.js b/packages/react-devtools-shell/src/app/SuspenseTree/index.js index 846e3f8ef6..c18a6315a6 100644 --- a/packages/react-devtools-shell/src/app/SuspenseTree/index.js +++ b/packages/react-devtools-shell/src/app/SuspenseTree/index.js @@ -12,6 +12,7 @@ import { Fragment, Suspense, unstable_SuspenseList as SuspenseList, + useReducer, useState, } from 'react'; @@ -26,10 +27,156 @@ function SuspenseTree(): React.Node { + ); } +function IgnoreMePassthrough({children}: {children: React$Node}) { + return {children}; +} + +const suspenseTreeOperationsChildren = { + a: ( + +

A

+
+ ), + b: ( +
+ B +
+ ), + c: ( +

+ + C + +

+ ), + d: ( + +
D
+
+ ), + e: ( + + + +

e1

+
+
+ + +
e2
+
+
+
+ ), + eReordered: ( + + + +
e2
+
+
+ + +

e1

+
+
+
+ ), +}; + +function SuspenseTreeOperations() { + const initialChildren: any[] = [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.e, + ]; + const [children, dispatch] = useReducer( + ( + pendingState: any[], + action: 'toggle-mount' | 'reorder' | 'reorder-within-filtered', + ): React$Node[] => { + switch (action) { + case 'toggle-mount': + if (pendingState.length === 5) { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + ]; + } else { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.e, + ]; + } + case 'reorder': + if (pendingState[1] === suspenseTreeOperationsChildren.b) { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.e, + ]; + } else { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.e, + ]; + } + case 'reorder-within-filtered': + if (pendingState[4] === suspenseTreeOperationsChildren.e) { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.eReordered, + ]; + } else { + return [ + suspenseTreeOperationsChildren.a, + suspenseTreeOperationsChildren.b, + suspenseTreeOperationsChildren.c, + suspenseTreeOperationsChildren.d, + suspenseTreeOperationsChildren.e, + ]; + } + default: + return pendingState; + } + }, + initialChildren, + ); + + return ( + <> + + + + +
{children}
+
+ + ); +} + function EmptySuspense() { return ; } @@ -144,7 +291,8 @@ function LoadLater() { setLoadChild(true)}>Click to load - }> + } + name="LoadLater"> {loadChild ? ( setLoadChild(false)}> Loaded! Click to suspend again. From 594fb5e9abea8aee20dec5e0dfba39dac16ce53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 01:50:26 -0400 Subject: [PATCH 03/24] [DevTools] Always skip 1 frame (#34167) Follow up to #34093. There's an issue where the skipFrames argument isn't part of the cache key so the other parsers that expect skipping one frame might skip zero and show the internal `fakeJSXDEV` callsite. Ideally we should include the skipFrames as part of the cache key but we can also always just skip one. --- .../react-devtools-shared/src/backend/utils/parseStackTrace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js b/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js index 92b4156de7..335fe42709 100644 --- a/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js +++ b/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js @@ -284,7 +284,7 @@ export function parseStackTrace( export function extractLocationFromOwnerStack( error: Error, ): ReactFunctionLocation | null { - const stackTrace = parseStackTrace(error, 0); + const stackTrace = parseStackTrace(error, 1); const stack = error.stack; if ( !stack.includes('react_stack_bottom_frame') && From 72965f361547da79fcd4310ae13e22d6abb274a6 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 11 Aug 2025 17:12:39 +0200 Subject: [PATCH 04/24] [DevTools] Restore reconciling Suspense stack after fallback was reconciled (#34168) --- .../src/backend/fiber/renderer.js | 70 ++++++++++--------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 236b31a3d9..bf28c22728 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4162,7 +4162,7 @@ export function attach( const stashedSuspenseParent = reconcilingParentSuspenseNode; const stashedSuspensePrevious = previouslyReconciledSiblingSuspenseNode; const stashedSuspenseRemaining = remainingReconcilingChildrenSuspenseNodes; - let shouldPopSuspenseNode = false; + let shouldMeasureSuspenseNode = false; let previousSuspendedBy = null; if (fiberInstance !== null) { previousSuspendedBy = fiberInstance.suspendedBy; @@ -4192,7 +4192,7 @@ export function attach( previouslyReconciledSiblingSuspenseNode = null; remainingReconcilingChildrenSuspenseNodes = suspenseNode.firstChild; suspenseNode.firstChild = null; - shouldPopSuspenseNode = true; + shouldMeasureSuspenseNode = true; } } try { @@ -4379,38 +4379,40 @@ export function attach( 0, ); - // 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. - // Since the fallback conceptually blocks the parent. - reconcilingParentSuspenseNode = stashedSuspenseParent; - previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; - remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; - shouldPopSuspenseNode = false; + shouldMeasureSuspenseNode = false; if (nextFallbackFiber !== null) { - updateFlags |= updateVirtualChildrenRecursively( - nextFallbackFiber, - null, - prevFallbackFiber, - traceNearestHostComponentUpdate, - 0, - ); - } else if ( - nextFiber.memoizedState === null && - fiberInstance.suspenseNode !== null - ) { - if (!isInDisconnectedSubtree) { - // 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 - // we're suspended to visualize the resulting state. - const suspenseNode = fiberInstance.suspenseNode; - const prevRects = suspenseNode.rects; - const nextRects = measureInstance(fiberInstance); - if (!areEqualRects(prevRects, nextRects)) { - suspenseNode.rects = nextRects; - recordSuspenseResize(suspenseNode); - } + 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. + // Since the fallback conceptually blocks the parent. + reconcilingParentSuspenseNode = stashedSuspenseParent; + previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; + remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; + try { + updateFlags |= updateVirtualChildrenRecursively( + nextFallbackFiber, + null, + prevFallbackFiber, + traceNearestHostComponentUpdate, + 0, + ); + } finally { + reconcilingParentSuspenseNode = fallbackStashedSuspenseParent; + previouslyReconciledSiblingSuspenseNode = + fallbackStashedSuspensePrevious; + remainingReconcilingChildrenSuspenseNodes = + fallbackStashedSuspenseRemaining; } + } else 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 + // we're suspended to visualize the resulting state. + shouldMeasureSuspenseNode = !isInDisconnectedSubtree; } } else { // Common case: Primary -> Primary. @@ -4519,7 +4521,7 @@ export function attach( reconcilingParent = stashedParent; previouslyReconciledSibling = stashedPrevious; remainingReconcilingChildren = stashedRemaining; - if (shouldPopSuspenseNode) { + if (shouldMeasureSuspenseNode) { if ( !isInDisconnectedSubtree && reconcilingParentSuspenseNode !== null @@ -4535,6 +4537,8 @@ export function attach( recordSuspenseResize(suspenseNode); } } + } + if (fiberInstance.suspenseNode !== null) { reconcilingParentSuspenseNode = stashedSuspenseParent; previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; From 59ef3c4baf5fa107955eb72c3ee0f6e01a9923be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:41:14 -0400 Subject: [PATCH 05/24] [DevTools] Allow Introspection of React Elements and React.lazy (#34129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With RSC it's common to get React.lazy objects in the children position. This first formats them nicely. Then it adds introspection support for both lazy and elements. Unfortunately because of quirks with the hydration mechanism we have to expose it under the name `_payload` instead of something direct. Also because the name "type" is taken we can't expose the type field on an element neither. That whole algorithm could use a rewrite. Screenshot 2025-08-07 at 11 37 03 PM Screenshot 2025-08-07 at 11 36 36 PM For JSX an alternative or additional feature might be instead to jump to the first Instance that was rendered using that JSX. We know that based on the equality of the memoizedProps on the Fiber. It's just a matter of whether we do that eagerly or more lazily when you click but you may not have a match so would be nice to indicate that before you click. --- .../src/__tests__/inspectedElement-test.js | 30 ++++-- .../__tests__/legacy/inspectElement-test.js | 10 +- .../react-devtools-shared/src/hydration.js | 102 ++++++++++++++++-- packages/react-devtools-shared/src/utils.js | 68 +++++++++++- 4 files changed, 185 insertions(+), 25 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 522d211aeb..1136dd2928 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -682,6 +682,7 @@ describe('InspectedElement', () => { object_with_symbol={objectWithSymbol} proxy={proxyInstance} react_element={} + react_lazy={React.lazy(async () => ({default: 'foo'}))} regexp={/abc/giu} set={setShallow} set_of_sets={setOfSets} @@ -780,9 +781,18 @@ describe('InspectedElement', () => { "preview_short": () => {}, "preview_long": () => {}, }, - "react_element": Dehydrated { - "preview_short": , - "preview_long": , + "react_element": { + "key": null, + "props": Dehydrated { + "preview_short": {…}, + "preview_long": {}, + }, + }, + "react_lazy": { + "_payload": Dehydrated { + "preview_short": {…}, + "preview_long": {_result: () => {}, _status: -1}, + }, }, "regexp": Dehydrated { "preview_short": /abc/giu, @@ -930,13 +940,13 @@ describe('InspectedElement', () => { const inspectedElement = await inspectElementAtIndex(0); expect(inspectedElement.props).toMatchInlineSnapshot(` - { - "unusedPromise": Dehydrated { - "preview_short": Promise, - "preview_long": Promise, - }, - } - `); + { + "unusedPromise": Dehydrated { + "preview_short": Promise, + "preview_long": Promise, + }, + } + `); }); it('should not consume iterables while inspecting', async () => { diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index cf1ce1ffa3..f306ab9709 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -289,9 +289,13 @@ describe('InspectedElementContext', () => { "preview_long": {boolean: true, number: 123, string: "abc"}, }, }, - "react_element": Dehydrated { - "preview_short": , - "preview_long": , + "react_element": { + "key": null, + "props": Dehydrated { + "preview_short": {…}, + "preview_long": {}, + }, + "ref": null, }, "regexp": Dehydrated { "preview_short": /abc/giu, diff --git a/packages/react-devtools-shared/src/hydration.js b/packages/react-devtools-shared/src/hydration.js index 7ce5a8ec6a..ecadad7ab3 100644 --- a/packages/react-devtools-shared/src/hydration.js +++ b/packages/react-devtools-shared/src/hydration.js @@ -16,6 +16,8 @@ import { setInObject, } from 'react-devtools-shared/src/utils'; +import {REACT_LEGACY_ELEMENT_TYPE} from 'shared/ReactSymbols'; + import type { DehydratedData, InspectedElementPath, @@ -188,18 +190,103 @@ export function dehydrate( type, }; - // React Elements aren't very inspector-friendly, - // and often contain private fields or circular references. - case 'react_element': - cleaned.push(path); - return { - inspectable: false, + case 'react_element': { + isPathAllowedCheck = isPathAllowed(path); + + if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) { + cleaned.push(path); + return { + inspectable: true, + preview_short: formatDataForPreview(data, false), + preview_long: formatDataForPreview(data, true), + name: getDisplayNameForReactElement(data) || 'Unknown', + type, + }; + } + + const unserializableValue: Unserializable = { + unserializable: true, + type, + readonly: true, preview_short: formatDataForPreview(data, false), preview_long: formatDataForPreview(data, true), name: getDisplayNameForReactElement(data) || 'Unknown', - type, }; + // TODO: We can't expose type because that name is already taken on Unserializable. + unserializableValue.key = dehydrate( + data.key, + cleaned, + unserializable, + path.concat(['key']), + isPathAllowed, + isPathAllowedCheck ? 1 : level + 1, + ); + if (data.$$typeof === REACT_LEGACY_ELEMENT_TYPE) { + unserializableValue.ref = dehydrate( + data.ref, + cleaned, + unserializable, + path.concat(['ref']), + isPathAllowed, + isPathAllowedCheck ? 1 : level + 1, + ); + } + unserializableValue.props = dehydrate( + data.props, + cleaned, + unserializable, + path.concat(['props']), + isPathAllowed, + isPathAllowedCheck ? 1 : level + 1, + ); + unserializable.push(path); + return unserializableValue; + } + case 'react_lazy': { + isPathAllowedCheck = isPathAllowed(path); + + const payload = data._payload; + + if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) { + cleaned.push(path); + const inspectable = + payload !== null && + typeof payload === 'object' && + (payload._status === 1 || + payload._status === 2 || + payload.status === 'fulfilled' || + payload.status === 'rejected'); + return { + inspectable, + preview_short: formatDataForPreview(data, false), + preview_long: formatDataForPreview(data, true), + name: 'lazy()', + type, + }; + } + + const unserializableValue: Unserializable = { + unserializable: true, + type: type, + preview_short: formatDataForPreview(data, false), + preview_long: formatDataForPreview(data, true), + name: 'lazy()', + }; + // Ideally we should alias these properties to something more readable but + // unfortunately because of how the hydration algorithm uses a single concept of + // "path" we can't alias the path. + unserializableValue._payload = dehydrate( + payload, + cleaned, + unserializable, + path.concat(['_payload']), + isPathAllowed, + isPathAllowedCheck ? 1 : level + 1, + ); + unserializable.push(path); + return unserializableValue; + } // ArrayBuffers error if you try to inspect them. case 'array_buffer': case 'data_view': @@ -309,6 +396,7 @@ export function dehydrate( isPathAllowedCheck = isPathAllowed(path); if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) { + cleaned.push(path); return { inspectable: data.status === 'fulfilled' || data.status === 'rejected', diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index ef5e7450ac..2c6d026cd8 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -633,6 +633,7 @@ export type DataType = | 'thenable' | 'object' | 'react_element' + | 'react_lazy' | 'regexp' | 'string' | 'symbol' @@ -686,11 +687,12 @@ export function getDataType(data: Object): DataType { return 'number'; } case 'object': - if ( - data.$$typeof === REACT_ELEMENT_TYPE || - data.$$typeof === REACT_LEGACY_ELEMENT_TYPE - ) { - return 'react_element'; + switch (data.$$typeof) { + case REACT_ELEMENT_TYPE: + case REACT_LEGACY_ELEMENT_TYPE: + return 'react_element'; + case REACT_LAZY_TYPE: + return 'react_lazy'; } if (isArray(data)) { return 'array'; @@ -906,6 +908,62 @@ export function formatDataForPreview( return `<${truncateForDisplay( getDisplayNameForReactElement(data) || 'Unknown', )} />`; + case 'react_lazy': + // To avoid actually initialize a lazy to cause a side-effect we make some assumptions + // about the structure of the payload even though that's not really part of the contract. + // In practice, this is really just coming from React.lazy helper or Flight. + const payload = data._payload; + if (payload !== null && typeof payload === 'object') { + if (payload._status === 0) { + // React.lazy constructor pending + return `pending lazy()`; + } + if (payload._status === 1 && payload._result != null) { + // React.lazy constructor fulfilled + if (showFormattedValue) { + const formatted = formatDataForPreview( + payload._result.default, + false, + ); + return `fulfilled lazy() {${truncateForDisplay(formatted)}}`; + } else { + return `fulfilled lazy() {…}`; + } + } + if (payload._status === 2) { + // React.lazy constructor rejected + if (showFormattedValue) { + const formatted = formatDataForPreview(payload._result, false); + return `rejected lazy() {${truncateForDisplay(formatted)}}`; + } else { + return `rejected lazy() {…}`; + } + } + if (payload.status === 'pending' || payload.status === 'blocked') { + // React Flight pending + return `pending lazy()`; + } + if (payload.status === 'fulfilled') { + // React Flight fulfilled + if (showFormattedValue) { + const formatted = formatDataForPreview(payload.value, false); + return `fulfilled lazy() {${truncateForDisplay(formatted)}}`; + } else { + return `fulfilled lazy() {…}`; + } + } + if (payload.status === 'rejected') { + // React Flight rejected + if (showFormattedValue) { + const formatted = formatDataForPreview(payload.reason, false); + return `rejected lazy() {${truncateForDisplay(formatted)}}`; + } else { + return `rejected lazy() {…}`; + } + } + } + // Some form of uninitialized + return 'lazy()'; case 'array_buffer': return `ArrayBuffer(${data.byteLength})`; case 'data_view': From 7a934a16b861366282e297ee61611f9bd8c524cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:41:30 -0400 Subject: [PATCH 06/24] [DevTools] Show Owner Stacks in "rendered by" View (#34130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This shows the stack trace of the JSX at each level so now you can also jump to the code location for the JSX callsite. The visual is similar to the owner stacks with `createTask` except when you click the `<...>` you jump to the Instance in the Components panel. Screenshot 2025-08-08 at 12 19 21 AM I'm not sure it's really necessary to have all the JSX stacks of every owner. We could just have it for the current component and then the rest of the owners you could get to if you just click that owner instance. As a bonus, I also use the JSX callsite as the fallback for the "View Source" button. This is primarily useful for built-ins like `
` and `` that don't have any implementation to jump to anyway. It's useful to be able to jump to where a boundary was defined. --- .../__tests__/__e2e__/devtools-utils.js | 15 ++++++-- .../src/__tests__/profilingCache-test.js | 1 + .../src/backend/fiber/renderer.js | 18 ++++++++++ .../src/backend/legacy/renderer.js | 3 ++ .../src/backend/types.js | 4 +++ .../react-devtools-shared/src/backendAPI.js | 2 ++ .../views/Components/InspectedElement.js | 20 +++++++---- .../views/Components/InspectedElementView.js | 35 ++++++++++++------- .../devtools/views/Components/OwnerView.js | 3 +- .../src/frontend/types.js | 4 +++ 10 files changed, 82 insertions(+), 23 deletions(-) diff --git a/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js b/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js index fe2bb3f6f2..c39f63dc5b 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js @@ -64,11 +64,22 @@ async function selectElement( createTestNameSelector('InspectedElementView-Owners'), ])[0]; + if (!ownersList) { + return false; + } + + const owners = findAllNodes(ownersList, [ + createTestNameSelector('OwnerView'), + ]); + return ( title && title.innerText.includes(titleText) && - ownersList && - ownersList.innerText.includes(ownersListText) + owners && + owners + .map(node => node.innerText) + .join('\n') + .includes(ownersListText) ); }, {titleText: displayName, ownersListText: waitForOwnersText} diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index 795f37183a..d16062c69f 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -949,6 +949,7 @@ describe('ProfilingCache', () => { "hocDisplayNames": null, "id": 1, "key": null, + "stack": null, "type": 11, }, ], diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index bf28c22728..5510100f0d 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4991,6 +4991,10 @@ export function attach( id: instance.id, key: fiber.key, env: null, + stack: + fiber._debugOwner == null || fiber._debugStack == null + ? null + : parseStackTrace(fiber._debugStack, 1), type: getElementTypeForFiber(fiber), }; } else { @@ -5000,6 +5004,10 @@ export function attach( id: instance.id, key: componentInfo.key == null ? null : componentInfo.key, env: componentInfo.env == null ? null : componentInfo.env, + stack: + componentInfo.owner == null || componentInfo.debugStack == null + ? null + : parseStackTrace(componentInfo.debugStack, 1), type: ElementTypeVirtual, }; } @@ -5598,6 +5606,11 @@ export function attach( source, + stack: + fiber._debugOwner == null || fiber._debugStack == null + ? null + : parseStackTrace(fiber._debugStack, 1), + // Does the component have legacy context attached to it. hasLegacyContext, @@ -5698,6 +5711,11 @@ export function attach( source, + stack: + componentInfo.owner == null || componentInfo.debugStack == null + ? null + : parseStackTrace(componentInfo.debugStack, 1), + // Does the component have legacy context attached to it. hasLegacyContext: false, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 6153e08832..faceec35a1 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -796,6 +796,7 @@ export function attach( id: getID(owner), key: element.key, env: null, + stack: null, type: getElementType(owner), }); if (owner._currentElement) { @@ -837,6 +838,8 @@ export function attach( source: null, + stack: null, + // Only legacy context exists in legacy versions. hasLegacyContext: true, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 585654252d..55a1bc6532 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -257,6 +257,7 @@ export type SerializedElement = { id: number, key: number | string | null, env: null | string, + stack: null | ReactStackTrace, type: ElementType, }; @@ -308,6 +309,9 @@ export type InspectedElement = { source: ReactFunctionLocation | null, + // The location of the JSX creation. + stack: ReactStackTrace | null, + type: ElementType, // Meta information about the root this element belongs to. diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index a27e70c26d..db22606377 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -257,6 +257,7 @@ export function convertInspectedElementBackendToFrontend( owners, env, source, + stack, context, hooks, plugins, @@ -295,6 +296,7 @@ export function convertInspectedElementBackendToFrontend( // Previous backend implementations (<= 6.1.5) have a different interface for Source. // This gates the source features for only compatible backends: >= 6.1.6 source: Array.isArray(source) ? source : null, + stack: stack, type, owners: owners === null 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 cc37953f4d..7b19908cc8 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -51,12 +51,19 @@ export default function InspectedElementWrapper(_: Props): React.Node { const fetchFileWithCaching = useContext(FetchFileWithCachingContext); + const source = + inspectedElement == null + ? null + : inspectedElement.source != null + ? inspectedElement.source + : inspectedElement.stack != null && inspectedElement.stack.length > 0 + ? inspectedElement.stack[0] + : null; + const symbolicatedSourcePromise: null | Promise = React.useMemo(() => { - if (inspectedElement == null) return null; if (fetchFileWithCaching == null) return Promise.resolve(null); - const {source} = inspectedElement; if (source == null) return Promise.resolve(null); const [, sourceURL, line, column] = source; @@ -66,7 +73,7 @@ export default function InspectedElementWrapper(_: Props): React.Node { line, column, ); - }, [inspectedElement]); + }, [source]); const element = inspectedElementID !== null @@ -223,13 +230,12 @@ export default function InspectedElementWrapper(_: Props): React.Node { {!alwaysOpenInEditor && !!editorURL && - inspectedElement != null && - inspectedElement.source != null && + source != null && symbolicatedSourcePromise != null && ( }> @@ -276,7 +282,7 @@ export default function InspectedElementWrapper(_: Props): React.Node { {!hideViewSourceAction && ( )} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js index 95f7aee68d..1318e96c30 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js @@ -22,6 +22,7 @@ import InspectedElementSuspendedBy from './InspectedElementSuspendedBy'; import NativeStyleEditor from './NativeStyleEditor'; import {enableStyleXFeatures} from 'react-devtools-feature-flags'; import InspectedElementSourcePanel from './InspectedElementSourcePanel'; +import StackTraceView from './StackTraceView'; import OwnerView from './OwnerView'; import styles from './InspectedElementView.css'; @@ -52,6 +53,7 @@ export default function InspectedElementView({ symbolicatedSourcePromise, }: Props): React.Node { const { + stack, owners, rendererPackageName, rendererVersion, @@ -68,8 +70,9 @@ export default function InspectedElementView({ ? `${rendererPackageName}@${rendererVersion}` : null; const showOwnersList = owners !== null && owners.length > 0; + const showStack = stack != null && stack.length > 0; const showRenderedBy = - showOwnersList || rendererLabel !== null || rootType !== null; + showStack || showOwnersList || rendererLabel !== null || rootType !== null; return ( @@ -168,20 +171,26 @@ export default function InspectedElementView({ data-testname="InspectedElementView-Owners">
rendered by
+ {showStack ? : null} {showOwnersList && owners?.map(owner => ( - + <> + + {owner.stack != null && owner.stack.length > 0 ? ( + + ) : null} + ))} {rootType !== null && ( diff --git a/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js index ac84848437..2b0f4b035a 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/OwnerView.js @@ -60,7 +60,8 @@ export default function OwnerView({ + title={displayName} + data-testname="OwnerView"> {'<' + displayName + '>'} diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 3fff08877c..e9bd9158b6 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -216,6 +216,7 @@ export type SerializedElement = { id: number, key: number | string | null, env: null | string, + stack: null | ReactStackTrace, hocDisplayNames: Array | null, compiledWithForget: boolean, type: ElementType, @@ -279,6 +280,9 @@ export type InspectedElement = { // Location of component in source code. source: ReactFunctionLocation | null, + // The location of the JSX creation. + stack: ReactStackTrace | null, + type: ElementType, // Meta information about the root this element belongs to. From ab5238d5a40a4a4a68b351d345ab26f8ec5785e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:41:46 -0400 Subject: [PATCH 07/24] [DevTools] Show name prop of Suspense / Activity in the Components Tree view (#34135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name prop will be used in the Suspense tab to help identity a boundary. Activity will also allow names. A custom component can be identified by the name of the component but built-ins doesn't have that. This PR adds it to the Components Tree View as well since otherwise you only have the key to go on. Normally we don't add all the props to avoid making this view too noisy but this is an exception along with key to help identify a boundary quickly in the tree. Unlike the SuspenseNode store, this wouldn't ever have a name inferred by owner since that kind of context already exists in this view. Screenshot 2025-08-08 at 1 20 36 PM I also made both the key and name prop searchable. Screenshot 2025-08-08 at 1 32 27 PM --- .../src/backend/fiber/renderer.js | 12 +++++++++++ .../src/backend/legacy/renderer.js | 1 + .../src/devtools/store.js | 6 ++++++ .../src/devtools/views/Components/Element.js | 20 ++++++++++++++++++- .../devtools/views/Components/TreeContext.js | 13 +++++++++++- .../views/Profiler/CommitTreeBuilder.js | 3 +++ .../src/frontend/types.js | 1 + packages/react-devtools-shared/src/utils.js | 1 + packages/shared/ReactTypes.js | 1 + 9 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 5510100f0d..3a3c254f5c 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2369,6 +2369,15 @@ export function attach( const keyString = key === null ? null : String(key); const keyStringID = getStringID(keyString); + const nameProp = + fiber.tag === SuspenseComponent + ? fiber.memoizedProps.name + : fiber.tag === ActivityComponent + ? fiber.memoizedProps.name + : null; + const namePropString = nameProp == null ? null : String(nameProp); + const namePropStringID = getStringID(namePropString); + pushOperation(TREE_OPERATION_ADD); pushOperation(id); pushOperation(elementType); @@ -2376,6 +2385,7 @@ export function attach( pushOperation(ownerID); pushOperation(displayNameStringID); pushOperation(keyStringID); + pushOperation(namePropStringID); // If this subtree has a new mode, let the frontend know. if ((fiber.mode & StrictModeBits) !== 0) { @@ -2478,6 +2488,7 @@ export function attach( // in such a way as to bypass the default stringification of the "key" property. const keyString = key === null ? null : String(key); const keyStringID = getStringID(keyString); + const namePropStringID = getStringID(null); const id = instance.id; @@ -2488,6 +2499,7 @@ export function attach( pushOperation(ownerID); pushOperation(displayNameStringID); pushOperation(keyStringID); + pushOperation(namePropStringID); const componentLogsEntry = componentInfoToComponentLogsMap.get(componentInfo); diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index faceec35a1..c2c2783936 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -426,6 +426,7 @@ export function attach( pushOperation(ownerID); pushOperation(displayNameStringID); pushOperation(keyStringID); + pushOperation(getStringID(null)); // name prop } } diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 622c9a4754..2d6b67ef12 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -1116,6 +1116,7 @@ export default class Store extends EventEmitter<{ isCollapsed: false, // Never collapse roots; it would hide the entire tree. isStrictModeNonCompliant, key: null, + nameProp: null, ownerID: 0, parentID: 0, type, @@ -1139,6 +1140,10 @@ export default class Store extends EventEmitter<{ const key = stringTable[keyStringID]; i++; + const namePropStringID = operations[i]; + const nameProp = stringTable[namePropStringID]; + i++; + if (__DEBUG__) { debug( 'Add', @@ -1180,6 +1185,7 @@ export default class Store extends EventEmitter<{ isCollapsed: this._collapseNodesByDefault, isStrictModeNonCompliant: parentElement.isStrictModeNonCompliant, key, + nameProp, ownerID, parentID, type, diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.js b/packages/react-devtools-shared/src/devtools/views/Components/Element.js index c3ddf1da07..25e5208ce9 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.js @@ -119,6 +119,7 @@ export default function Element({data, index, style}: Props): React.Node { hocDisplayNames, isStrictModeNonCompliant, key, + nameProp, compiledWithForget, } = element; const { @@ -179,7 +180,24 @@ export default function Element({data, index, style}: Props): React.Node { className={styles.KeyValue} title={key} onDoubleClick={handleKeyDoubleClick}> -
{key}
+
+                
+              
+
+ " +
+ )} + + {nameProp && ( + +  name=" + +
+                
+              
"
diff --git a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js index f43ced8244..72556543f4 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js @@ -995,7 +995,14 @@ function recursivelySearchTree( return; } - const {children, displayName, hocDisplayNames, compiledWithForget} = element; + const { + children, + displayName, + hocDisplayNames, + compiledWithForget, + key, + nameProp, + } = element; if (displayName != null && regExp.test(displayName) === true) { searchResults.push(elementID); } else if ( @@ -1006,6 +1013,10 @@ function recursivelySearchTree( searchResults.push(elementID); } else if (compiledWithForget && regExp.test('Forget')) { searchResults.push(elementID); + } else if (typeof key === 'string' && regExp.test(key)) { + searchResults.push(elementID); + } else if (typeof nameProp === 'string' && regExp.test(nameProp)) { + searchResults.push(elementID); } children.forEach(childID => diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js index dfa515fffa..d685263a22 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -239,6 +239,9 @@ function updateTree( const key = stringTable[keyStringID]; i++; + // skip name prop + i++; + if (__DEBUG__) { debug( 'Add', diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index e9bd9158b6..0089059df9 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -157,6 +157,7 @@ export type Element = { type: ElementType, displayName: string | null, key: number | string | null, + nameProp: null | string, hocDisplayNames: null | Array, diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 2c6d026cd8..c585d90500 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -271,6 +271,7 @@ export function printOperationsArray(operations: Array) { i++; i++; // key + i++; // name logs.push( `Add node ${id} (${displayName || 'null'}) as child of ${parentID}`, diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 5c7af1d1b3..af514a9510 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -298,6 +298,7 @@ export type ViewTransitionProps = { export type ActivityProps = { mode?: 'hidden' | 'visible' | null | void, children?: ReactNodeList, + name?: string, }; export type SuspenseProps = { From 6445b3154ee60c2b2aa15d8be437a3f07feeb8f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:42:23 -0400 Subject: [PATCH 08/24] [Fiber] Add additional debugInfo to React.lazy constructors in DEV (#34137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This creates a debug info object for the React.lazy call when it's called on the client. We have some additional information we can track for these since they're created by React earlier. We can track the stack trace where `React.lazy` was called to associate it back to something useful. We can track the start time when we initialized it for the first time and the end time when it resolves. The name from the promise if available. This data is currently only picked up in child position and not component position. The component position is in a follow up. Screenshot 2025-08-08 at 2 49 33 PM This begs for ignore listing in the front end since these stacks aren't filtered on the server. --- .../src/__tests__/ReactFlight-test.js | 18 ++-- packages/react/src/ReactLazy.js | 87 ++++++++++++++++++- packages/shared/ReactTypes.js | 1 + 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 9a60c3bd66..0fd9b869c6 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2822,7 +2822,7 @@ describe('ReactFlight', () => { expect(getDebugInfo(promise)).toEqual( __DEV__ ? [ - {time: 20}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 22 : 20}, { name: 'ServerComponent', env: 'Server', @@ -2832,7 +2832,7 @@ describe('ReactFlight', () => { transport: expect.arrayContaining([]), }, }, - {time: 21}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 23 : 21}, ] : undefined, ); @@ -2843,7 +2843,7 @@ describe('ReactFlight', () => { expect(getDebugInfo(thirdPartyChildren[0])).toEqual( __DEV__ ? [ - {time: 22}, // Clamped to the start + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, // Clamped to the start { name: 'ThirdPartyComponent', env: 'third-party', @@ -2851,15 +2851,15 @@ describe('ReactFlight', () => { stack: ' in Object. (at **)', props: {}, }, - {time: 22}, - {time: 23}, // This last one is when the promise resolved into the first party. + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 25 : 23}, // This last one is when the promise resolved into the first party. ] : undefined, ); expect(getDebugInfo(thirdPartyChildren[1])).toEqual( __DEV__ ? [ - {time: 22}, // Clamped to the start + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, // Clamped to the start { name: 'ThirdPartyLazyComponent', env: 'third-party', @@ -2867,14 +2867,14 @@ describe('ReactFlight', () => { stack: ' in myLazy (at **)\n in lazyInitializer (at **)', props: {}, }, - {time: 22}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, ] : undefined, ); expect(getDebugInfo(thirdPartyChildren[2])).toEqual( __DEV__ ? [ - {time: 22}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, { name: 'ThirdPartyFragmentComponent', env: 'third-party', @@ -2882,7 +2882,7 @@ describe('ReactFlight', () => { stack: ' in Object. (at **)', props: {}, }, - {time: 22}, + {time: gate(flags => flags.enableAsyncDebugInfo) ? 24 : 22}, ] : undefined, ); diff --git a/packages/react/src/ReactLazy.js b/packages/react/src/ReactLazy.js index 2ac29c8777..69b35b58cc 100644 --- a/packages/react/src/ReactLazy.js +++ b/packages/react/src/ReactLazy.js @@ -7,7 +7,16 @@ * @flow */ -import type {Wakeable, Thenable, ReactDebugInfo} from 'shared/ReactTypes'; +import type { + Wakeable, + Thenable, + FulfilledThenable, + RejectedThenable, + ReactDebugInfo, + ReactIOInfo, +} from 'shared/ReactTypes'; + +import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags'; import {REACT_LAZY_TYPE} from 'shared/ReactSymbols'; @@ -19,21 +28,25 @@ const Rejected = 2; type UninitializedPayload = { _status: -1, _result: () => Thenable<{default: T, ...}>, + _ioInfo?: ReactIOInfo, // DEV-only }; type PendingPayload = { _status: 0, _result: Wakeable, + _ioInfo?: ReactIOInfo, // DEV-only }; type ResolvedPayload = { _status: 1, _result: {default: T, ...}, + _ioInfo?: ReactIOInfo, // DEV-only }; type RejectedPayload = { _status: 2, _result: mixed, + _ioInfo?: ReactIOInfo, // DEV-only }; type Payload = @@ -51,6 +64,14 @@ export type LazyComponent = { function lazyInitializer(payload: Payload): T { if (payload._status === Uninitialized) { + if (__DEV__ && enableAsyncDebugInfo) { + const ioInfo = payload._ioInfo; + if (ioInfo != null) { + // Mark when we first kicked off the lazy request. + // $FlowFixMe[cannot-write] + ioInfo.start = ioInfo.end = performance.now(); + } + } const ctor = payload._result; const thenable = ctor(); // Transition to the next state. @@ -68,6 +89,21 @@ function lazyInitializer(payload: Payload): T { const resolved: ResolvedPayload = (payload: any); resolved._status = Resolved; resolved._result = moduleObject; + if (__DEV__) { + const ioInfo = payload._ioInfo; + if (ioInfo != null) { + // Mark the end time of when we resolved. + // $FlowFixMe[cannot-write] + ioInfo.end = performance.now(); + } + // Make the thenable introspectable + if (thenable.status === undefined) { + const fulfilledThenable: FulfilledThenable<{default: T, ...}> = + (thenable: any); + fulfilledThenable.status = 'fulfilled'; + fulfilledThenable.value = moduleObject; + } + } } }, error => { @@ -79,9 +115,37 @@ function lazyInitializer(payload: Payload): T { const rejected: RejectedPayload = (payload: any); rejected._status = Rejected; rejected._result = error; + if (__DEV__ && enableAsyncDebugInfo) { + const ioInfo = payload._ioInfo; + if (ioInfo != null) { + // Mark the end time of when we rejected. + // $FlowFixMe[cannot-write] + ioInfo.end = performance.now(); + } + // Make the thenable introspectable + if (thenable.status === undefined) { + const rejectedThenable: RejectedThenable<{default: T, ...}> = + (thenable: any); + rejectedThenable.status = 'rejected'; + rejectedThenable.reason = error; + } + } } }, ); + if (__DEV__ && enableAsyncDebugInfo) { + const ioInfo = payload._ioInfo; + if (ioInfo != null) { + // Stash the thenable for introspection of the value later. + // $FlowFixMe[cannot-write] + ioInfo.value = thenable; + const displayName = thenable.displayName; + if (typeof displayName === 'string') { + // $FlowFixMe[cannot-write] + ioInfo.name = displayName; + } + } + } if (payload._status === Uninitialized) { // In case, we're still uninitialized, then we're waiting for the thenable // to resolve. Set it as pending in the meantime. @@ -140,5 +204,26 @@ export function lazy( _init: lazyInitializer, }; + if (__DEV__ && enableAsyncDebugInfo) { + // TODO: We should really track the owner here but currently ReactIOInfo + // can only contain ReactComponentInfo and not a Fiber. It's unusual to + // create a lazy inside an owner though since they should be in module scope. + const owner = null; + const ioInfo: ReactIOInfo = { + name: 'lazy', + start: -1, + end: -1, + value: null, + owner: owner, + debugStack: new Error('react-stack-top-frame'), + // eslint-disable-next-line react-internal/no-production-logging + debugTask: console.createTask ? console.createTask('lazy()') : null, + }; + payload._ioInfo = ioInfo; + // Add debug info to the lazy, but this doesn't have an await stack yet. + // That will be inferred by later usage. + lazyType._debugInfo = [{awaited: ioInfo}]; + } + return lazyType; } diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index af514a9510..ff2649a23d 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -108,6 +108,7 @@ interface ThenableImpl { onFulfill: (value: T) => mixed, onReject: (error: mixed) => mixed, ): void | Wakeable; + displayName?: string; } interface UntrackedThenable extends ThenableImpl { status?: void; From 34ce3acafdcfb6830250043b64d8af2450c730bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:42:59 -0400 Subject: [PATCH 09/24] [DevTools] Pick up suspended by info from React.lazy in type position (#34144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normally, we pick up debug info from instrumented Promise or React.Lazy while we're reconciling in ReactChildFiber when they appear in the child position. We add those to the `_debugInfo` of the Fiber. However, we don't do that for for Lazy in the Component type position. Instead, we have to pick up the debug info from it explicitly in DevTools. Likely this is the info added by #34137. Older versions wouldn't be covered by this particular mechanism but more generally from throwing a Promise. Screenshot 2025-08-08 at 11 32 33 PM --- .../src/backend/fiber/renderer.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 3a3c254f5c..6fb5a66c7d 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -104,6 +104,7 @@ import { MEMO_NUMBER, MEMO_SYMBOL_STRING, SERVER_CONTEXT_SYMBOL_STRING, + LAZY_SYMBOL_STRING, } from '../shared/ReactSymbols'; import {enableStyleXFeatures} from 'react-devtools-feature-flags'; @@ -3161,6 +3162,25 @@ export function attach( return null; } + function trackDebugInfoFromLazyType(fiber: Fiber): void { + // The debugInfo from a Lazy isn't propagated onto _debugInfo of the parent Fiber the way + // it is when used in child position. So we need to pick it up explicitly. + const type = fiber.elementType; + const typeSymbol = getTypeSymbol(type); // The elementType might be have been a LazyComponent. + if (typeSymbol === LAZY_SYMBOL_STRING) { + const debugInfo: ?ReactDebugInfo = type._debugInfo; + if (debugInfo) { + for (let i = 0; i < debugInfo.length; i++) { + const debugEntry = debugInfo[i]; + if (debugEntry.awaited) { + const asyncInfo: ReactAsyncInfo = (debugEntry: any); + insertSuspendedBy(asyncInfo); + } + } + } + } + } + function mountVirtualChildrenRecursively( firstChild: Fiber, lastChild: null | Fiber, // non-inclusive @@ -3379,6 +3399,8 @@ export function attach( // because we don't want to highlight every host node inside of a newly mounted subtree. } + trackDebugInfoFromLazyType(fiber); + if (fiber.tag === HostHoistable) { const nearestInstance = reconcilingParent; if (nearestInstance === null) { @@ -4208,6 +4230,8 @@ export function attach( } } try { + trackDebugInfoFromLazyType(nextFiber); + if ( nextFiber.tag === HostHoistable && prevFiber.memoizedState !== nextFiber.memoizedState From 53d07944df70781b929f733e2059df43ca82edd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:44:05 -0400 Subject: [PATCH 10/24] [Fiber] Assign implicit debug info to used thenables (#34146) Similar to #34137 but for Promises. This lets us pick up the debug info from a raw Promise as a child which is not covered by `_debugThenables`. Currently ChildFiber doesn't stash its thenables so we can't pick them up from devtools after the fact without some debug info added to the parent. It also lets us track some approximate start/end time of use():ed promises based on the first time we saw this particular Promise. --- .../src/ReactFiberThenable.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index f4ae1d45b2..643be63ffa 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -12,6 +12,7 @@ import type { PendingThenable, FulfilledThenable, RejectedThenable, + ReactIOInfo, } from 'shared/ReactTypes'; import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; @@ -22,6 +23,8 @@ import {getWorkInProgressRoot} from './ReactFiberWorkLoop'; import ReactSharedInternals from 'shared/ReactSharedInternals'; +import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags'; + import noop from 'shared/noop'; opaque type ThenableStateDev = { @@ -154,6 +157,33 @@ export function trackUsedThenable( } } + if (__DEV__ && enableAsyncDebugInfo && thenable._debugInfo === undefined) { + // In DEV mode if the thenable that we observed had no debug info, then we add + // an inferred debug info so that we're able to track its potential I/O uniquely. + // We don't know the real start time since the I/O could have started much + // earlier and this could even be a cached Promise. Could be misleading. + const startTime = performance.now(); + const displayName = thenable.displayName; + const ioInfo: ReactIOInfo = { + name: typeof displayName === 'string' ? displayName : 'Promise', + start: startTime, + end: startTime, + value: (thenable: any), + // We don't know the requesting owner nor stack. + }; + // We can infer the await owner/stack lazily from where this promise ends up + // used. It can be used in more than one place so we can't assign it here. + thenable._debugInfo = [{awaited: ioInfo}]; + // Track when we resolved the Promise as the approximate end time. + if (thenable.status !== 'fulfilled' && thenable.status !== 'rejected') { + const trackEndTime = () => { + // $FlowFixMe[cannot-write] + ioInfo.end = performance.now(); + }; + thenable.then(trackEndTime, trackEndTime); + } + } + // We use an expando to track the status and result of a thenable so that we // can synchronously unwrap the value. Think of this as an extension of the // Promise API, or a custom interface that is a superset of Thenable. From 62a634b9722aa77d7ddb1fe8017db7d3a0925389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:46:27 -0400 Subject: [PATCH 11/24] [DebugTools] Use thenables from the _debugThenableState if available (#34161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the case where a Promise is not cached, then the thenable state might contain an older version. This version is the one that was actually observed by the committed render, so that's the version we'll want to inspect. We used to not store the thenable state but now we have it on `_debugThenableState` in DEV. Screenshot 2025-08-10 at 8 26 04 PM --- .../react-debug-tools/src/ReactDebugHooks.js | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 8242b27d4e..54a6dd3e43 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -147,6 +147,8 @@ function getPrimitiveStackCache(): Map> { let currentFiber: null | Fiber = null; let currentHook: null | Hook = null; let currentContextDependency: null | ContextDependency = null; +let currentThenableIndex: number = 0; +let currentThenableState: null | Array> = null; function nextHook(): null | Hook { const hook = currentHook; @@ -201,7 +203,15 @@ function use(usable: Usable): T { if (usable !== null && typeof usable === 'object') { // $FlowFixMe[method-unbinding] if (typeof usable.then === 'function') { - const thenable: Thenable = (usable: any); + const thenable: Thenable = + // If we have thenable state, then the actually used thenable will be the one + // stashed in it. It's possible for uncached Promises to be new each render + // and in that case the one we're inspecting is the in the thenable state. + currentThenableState !== null && + currentThenableIndex < currentThenableState.length + ? currentThenableState[currentThenableIndex++] + : (usable: any); + switch (thenable.status) { case 'fulfilled': { const fulfilledValue: T = thenable.value; @@ -1285,6 +1295,14 @@ export function inspectHooksOfFiber( // current state from them. currentHook = (fiber.memoizedState: Hook); currentFiber = fiber; + const thenableState = + fiber.dependencies && fiber.dependencies._debugThenableState; + // In DEV the thenableState is an inner object. + const usedThenables: any = thenableState + ? thenableState.thenables || thenableState + : null; + currentThenableState = Array.isArray(usedThenables) ? usedThenables : null; + currentThenableIndex = 0; if (hasOwnProperty.call(currentFiber, 'dependencies')) { // $FlowFixMe[incompatible-use]: Flow thinks hasOwnProperty might have nulled `currentFiber` @@ -1339,6 +1357,8 @@ export function inspectHooksOfFiber( currentFiber = null; currentHook = null; currentContextDependency = null; + currentThenableState = null; + currentThenableIndex = 0; restoreContexts(contextMap); } From ca292f7a57e8c5950cda51f1aa00509dbb07dbf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 11:48:09 -0400 Subject: [PATCH 12/24] [DevTools] Don't show "awaited by" if there's nothing to show (#34163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E.g. if the owner is null or the same as current component and no stack. This happens for example when you return a plain Promise in the child position and inspect the component it was returned in since there's no hook stack and the owner is the same as the instance itself so there's nothing new to link to. Before: Screenshot 2025-08-10 at 10 28 32 PM After: Screenshot 2025-08-10 at 10 29 04 PM --- .../views/Components/InspectedElementSuspendedBy.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 c24dd881e9..3608cff85c 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -104,11 +104,15 @@ function SuspendedByRow({ // Only show the awaited stack if the I/O started in a different owner // than where it was awaited. If it's started by the same component it's // probably easy enough to infer and less noise in the common case. + const canShowAwaitStack = + (asyncInfo.stack !== null && asyncInfo.stack.length > 0) || + (asyncOwner !== null && asyncOwner.id !== inspectedElement.id); const showAwaitStack = - !showIOStack || - (ioOwner === null - ? asyncOwner !== null - : asyncOwner === null || ioOwner.id !== asyncOwner.id); + canShowAwaitStack && + (!showIOStack || + (ioOwner === null + ? asyncOwner !== null + : asyncOwner === null || ioOwner.id !== asyncOwner.id)); const value: any = ioInfo.value; const metaName = From d587434c350a9ba317285ce7b535add47ab3c205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 12:10:05 -0400 Subject: [PATCH 13/24] [DevTools] Pick up suspended by info from use() (#34148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to #34144 but for `use()`. `use()` dependencies don't get added to the `fiber._debugInfo` set because that just models the things blocking the children, and not the Fiber component itself. This picks up any debug info from the thenable state that we stashed onto `_debugThenableState` so that we know it used `use()`. Screenshot 2025-08-09 at 4 03 40 PM Without #34146 this doesn't pick up uninstrumented promises but after it, it'll pick those up as well. An instrumented promise that doesn't have anything in its debug info is not picked up. For example, if it didn't depend on any I/O on the server. This doesn't yet pick up the stack trace of the `use()` call. That information is in the Hooks information but needs a follow up to extract it. --- .../src/__tests__/inspectedElement-test.js | 2 +- .../src/backend/fiber/renderer.js | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 1136dd2928..09f811172f 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -791,7 +791,7 @@ describe('InspectedElement', () => { "react_lazy": { "_payload": Dehydrated { "preview_short": {…}, - "preview_long": {_result: () => {}, _status: -1}, + "preview_long": {_ioInfo: {…}, _result: () => {}, _status: -1}, }, }, "regexp": Dehydrated { diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 6fb5a66c7d..41436e8a6e 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -8,6 +8,7 @@ */ import type { + Thenable, ReactComponentInfo, ReactDebugInfo, ReactAsyncInfo, @@ -3181,6 +3182,39 @@ export function attach( } } + function trackDebugInfoFromUsedThenables(fiber: Fiber): void { + // If a Fiber called use() in DEV mode then we may have collected _debugThenableState on + // the dependencies. If so, then this will contain the thenables passed to use(). + // These won't have their debug info picked up by fiber._debugInfo since that just + // contains things suspending the children. We have to collect use() separately. + const dependencies = fiber.dependencies; + if (dependencies == null) { + return; + } + const thenableState = dependencies._debugThenableState; + if (thenableState == null) { + return; + } + // In DEV the thenableState is an inner object. + const usedThenables: any = thenableState.thenables || thenableState; + if (!Array.isArray(usedThenables)) { + return; + } + for (let i = 0; i < usedThenables.length; i++) { + const thenable: Thenable = usedThenables[i]; + const debugInfo = thenable._debugInfo; + if (debugInfo) { + for (let j = 0; j < debugInfo.length; j++) { + const debugEntry = debugInfo[i]; + if (debugEntry.awaited) { + const asyncInfo: ReactAsyncInfo = (debugEntry: any); + insertSuspendedBy(asyncInfo); + } + } + } + } + } + function mountVirtualChildrenRecursively( firstChild: Fiber, lastChild: null | Fiber, // non-inclusive @@ -3400,6 +3434,7 @@ export function attach( } trackDebugInfoFromLazyType(fiber); + trackDebugInfoFromUsedThenables(fiber); if (fiber.tag === HostHoistable) { const nearestInstance = reconcilingParent; @@ -4231,6 +4266,7 @@ export function attach( } try { trackDebugInfoFromLazyType(nextFiber); + trackDebugInfoFromUsedThenables(nextFiber); if ( nextFiber.tag === HostHoistable && From f1e70b5e0aeffeba634f05a1524bf083f0340d5a Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 11 Aug 2025 12:13:33 -0400 Subject: [PATCH 14/24] [easy] remove leftover reference to disableDefaultPropsExceptForClasses (#34169) Noticed that I missed this in some earlier cleanup diff. Test Plan: grep for disableDefaultPropsExceptForClasses --- .../shared/forks/ReactFeatureFlags.test-renderer.native-fb.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 52a85eec8c..9cd9ac4ab5 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -13,7 +13,6 @@ import typeof * as ExportsType from './ReactFeatureFlags.test-renderer'; export const alwaysThrottleRetries = false; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; -export const disableDefaultPropsExceptForClasses = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; export const disableLegacyContextForFunctionComponents = false; From 2c9a42dfd7dc6f26be9694ffc6cb2ebf8ef8472b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 12:28:10 -0400 Subject: [PATCH 15/24] [DevTools] If the await doesn't have a stack use the stack from use() if any (#34162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #34148. This picks up the stack for the await from the `use()` Hook if one was used to get this async info. When you select a component that used hooks, we already collect this information. If you select a Suspense boundary, this lazily invokes the first component that awaited this data to inspects its hooks and produce a stack trace for the use(). When all we have for the name is "Promise" I also use the name of the first callsite in the stack trace if there's more than one. Which in practice will be the name of the custom Hook that called it. Ideally we'd use source mapping and ignore listing for this but that would require suspending the display. We could maybe make the SuspendedByRow wrapped in a Suspense boundary for this case. Screenshot 2025-08-10 at 10 07 55 PM --- .../src/backend/fiber/renderer.js | 162 ++++++++++++++---- .../Components/InspectedElementSuspendedBy.js | 16 +- 2 files changed, 146 insertions(+), 32 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 41436e8a6e..7634c6c472 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -13,8 +13,12 @@ import type { ReactDebugInfo, ReactAsyncInfo, ReactIOInfo, + ReactStackTrace, + ReactCallSite, } from 'shared/ReactTypes'; +import type {HooksTree} from 'react-debug-tools/src/ReactDebugHooks'; + import { ComponentFilterDisplayName, ComponentFilterElementType, @@ -5187,6 +5191,32 @@ export function attach( return null; } + function inspectHooks(fiber: Fiber): HooksTree { + const originalConsoleMethods: {[string]: $FlowFixMe} = {}; + + // Temporarily disable all console logging before re-running the hook. + for (const method in console) { + try { + // $FlowFixMe[invalid-computed-prop] + originalConsoleMethods[method] = console[method]; + // $FlowFixMe[prop-missing] + console[method] = () => {}; + } catch (error) {} + } + + try { + return inspectHooksOfFiber(fiber, getDispatcherRef(renderer)); + } finally { + // Restore original console functionality. + for (const method in originalConsoleMethods) { + try { + // $FlowFixMe[prop-missing] + console[method] = originalConsoleMethods[method]; + } catch (error) {} + } + } + } + function getSuspendedByOfSuspenseNode( suspenseNode: SuspenseNode, ): Array { @@ -5196,6 +5226,11 @@ export function attach( if (!suspenseNode.hasUniqueSuspenders) { return result; } + // Cache the inspection of Hooks in case we need it for multiple entries. + // We don't need a full map here since it's likely that every ioInfo that's unique + // to a specific instance will have those appear in order of when that instance was discovered. + let hooksCacheKey: null | DevToolsInstance = null; + let hooksCache: null | HooksTree = null; suspenseNode.suspendedBy.forEach((set, ioInfo) => { let parentNode = suspenseNode.parent; while (parentNode !== null) { @@ -5217,18 +5252,100 @@ export function attach( ioInfo, ); if (asyncInfo !== null) { - const index = result.length; - result.push(serializeAsyncInfo(asyncInfo, index, firstInstance)); + let hooks: null | HooksTree = null; + if (asyncInfo.stack == null && asyncInfo.owner == null) { + if (hooksCacheKey === firstInstance) { + hooks = hooksCache; + } else if (firstInstance.kind !== VIRTUAL_INSTANCE) { + const fiber = firstInstance.data; + if ( + fiber.dependencies && + fiber.dependencies._debugThenableState + ) { + // This entry had no stack nor owner but this Fiber used Hooks so we might + // be able to get the stack from the Hook. + hooksCacheKey = firstInstance; + hooksCache = hooks = inspectHooks(fiber); + } + } + } + result.push(serializeAsyncInfo(asyncInfo, firstInstance, hooks)); } } }); return result; } + function getAwaitStackFromHooks( + hooks: HooksTree, + asyncInfo: ReactAsyncInfo, + ): null | ReactStackTrace { + // TODO: We search through the hooks tree generated by inspectHooksOfFiber so that we can + // use the information already extracted but ideally this search would be faster since we + // could know which index to extract from the debug state. + for (let i = 0; i < hooks.length; i++) { + const node = hooks[i]; + const debugInfo = node.debugInfo; + if (debugInfo != null && debugInfo.indexOf(asyncInfo) !== -1) { + // Found a matching Hook. We'll now use its source location to construct a stack. + const source = node.hookSource; + if ( + source != null && + source.functionName !== null && + source.fileName !== null && + source.lineNumber !== null && + source.columnNumber !== null + ) { + // Unfortunately this is in a slightly different format. TODO: Unify HookNode with ReactCallSite. + const callSite: ReactCallSite = [ + source.functionName, + source.fileName, + source.lineNumber, + source.columnNumber, + 0, + 0, + false, + ]; + // As we return we'll add any custom hooks parent stacks to the array. + return [callSite]; + } else { + return []; + } + } + // Otherwise, search the sub hooks of any custom hook. + const matchedStack = getAwaitStackFromHooks(node.subHooks, asyncInfo); + if (matchedStack !== null) { + // Append this custom hook to the stack trace since it must have been called inside of it. + const source = node.hookSource; + if ( + source != null && + source.functionName !== null && + source.fileName !== null && + source.lineNumber !== null && + source.columnNumber !== null + ) { + // Unfortunately this is in a slightly different format. TODO: Unify HookNode with ReactCallSite. + const callSite: ReactCallSite = [ + source.functionName, + source.fileName, + source.lineNumber, + source.columnNumber, + 0, + 0, + false, + ]; + matchedStack.push(callSite); + } + return matchedStack; + } + } + return null; + } + function serializeAsyncInfo( asyncInfo: ReactAsyncInfo, - index: number, parentInstance: DevToolsInstance, + hooks: null | HooksTree, ): SerializedAsyncInfo { const ioInfo = asyncInfo.awaited; const ioOwnerInstance = findNearestOwnerInstance( @@ -5268,6 +5385,11 @@ export function attach( // If we awaited in the child position of a component, then the best stack would be the // return callsite but we don't have that available so instead we skip. The callsite of // the JSX would be misleading in this case. The same thing happens with throw-a-Promise. + if (hooks !== null) { + // If this component used Hooks we might be able to instead infer the stack from the + // use() callsite if this async info came from a hook. Let's search the tree to find it. + awaitStack = getAwaitStackFromHooks(hooks, asyncInfo); + } break; default: // If we awaited by passing a Promise to a built-in element, then the JSX callsite is a @@ -5538,31 +5660,9 @@ export function attach( const owners: null | Array = getOwnersListFromInstance(fiberInstance); - let hooks = null; + let hooks: null | HooksTree = null; if (usesHooks) { - const originalConsoleMethods: {[string]: $FlowFixMe} = {}; - - // Temporarily disable all console logging before re-running the hook. - for (const method in console) { - try { - // $FlowFixMe[invalid-computed-prop] - originalConsoleMethods[method] = console[method]; - // $FlowFixMe[prop-missing] - console[method] = () => {}; - } catch (error) {} - } - - try { - hooks = inspectHooksOfFiber(fiber, getDispatcherRef(renderer)); - } finally { - // Restore original console functionality. - for (const method in originalConsoleMethods) { - try { - // $FlowFixMe[prop-missing] - console[method] = originalConsoleMethods[method]; - } catch (error) {} - } - } + hooks = inspectHooks(fiber); } let rootType = null; @@ -5641,8 +5741,8 @@ export function attach( // TODO: Prepend other suspense sources like css, images and use(). fiberInstance.suspendedBy === null ? [] - : fiberInstance.suspendedBy.map((info, index) => - serializeAsyncInfo(info, index, fiberInstance), + : fiberInstance.suspendedBy.map(info => + serializeAsyncInfo(info, fiberInstance, hooks), ); return { id: fiberInstance.id, @@ -5813,8 +5913,8 @@ export function attach( suspendedBy: suspendedBy === null ? [] - : suspendedBy.map((info, index) => - serializeAsyncInfo(info, index, virtualInstance), + : suspendedBy.map(info => + serializeAsyncInfo(info, virtualInstance, null), ), // List of owners 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 3608cff85c..da74bc579e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -81,7 +81,21 @@ function SuspendedByRow({ }: RowProps) { const [isOpen, setIsOpen] = useState(false); const ioInfo = asyncInfo.awaited; - const name = ioInfo.name; + let name = ioInfo.name; + if (name === '' || name === 'Promise') { + // If all we have is a generic name, we can try to infer a better name from + // the stack. We only do this if the stack has more than one frame since + // otherwise it's likely to just be the name of the component which isn't better. + const bestStack = ioInfo.stack || asyncInfo.stack; + if (bestStack !== null && bestStack.length > 1) { + // TODO: Ideally we'd get the name from the last ignore listed frame before the + // first visible frame since this is the same algorithm as the Flight server uses. + // Ideally, we'd also get the name from the source mapped entry instead of the + // original entry. However, that would require suspending the immediate display + // of these rows to first do source mapping before we can show the name. + name = bestStack[0][0]; + } + } const description = ioInfo.description; const longName = description === '' ? name : name + ' (' + description + ')'; const shortDescription = getShortDescription(name, description); From 3c67bbe5f90dbe78d0cd4db198db41978da4284e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 11 Aug 2025 12:28:32 -0400 Subject: [PATCH 16/24] [DevTools] Track suspensey CSS on "suspended by" (#34166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to track that Suspensey CSS (Host Resources) can contribute to the loading state. We can pick up the start/end time from the Performance Observer API since we know which resource was loaded. If DOM nodes are not filtered there's a link to the `` instance. The `"awaited by"` stack is the callsite of the JSX creating the ``. Screenshot 2025-08-11 at 1 35 21 AM Inspecting the link itself: Screenshot 2025-08-11 at 1 31 43 AM In this approach I only include it if the page currently matches the media query. It might contribute in some other scenario but we're not showing every possible state but every possible scenario that might suspend if timing changes in the current state. --- .../src/backend/fiber/renderer.js | 90 +++++++++++++++++++ .../Components/InspectedElementSuspendedBy.js | 9 +- packages/shared/ReactIODescription.js | 2 + 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 7634c6c472..1d4541253f 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -3219,6 +3219,94 @@ export function attach( } } + const hostAsyncInfoCache: WeakMap<{...}, ReactAsyncInfo> = new WeakMap(); + + function trackDebugInfoFromHostResource( + devtoolsInstance: DevToolsInstance, + fiber: Fiber, + ): void { + const resource: ?{ + type: 'stylesheet' | 'style' | 'script' | 'void', + instance?: null | HostInstance, + ... + } = fiber.memoizedState; + if (resource == null) { + return; + } + + // Use a cached entry based on the resource. This ensures that if we use the same + // resource in multiple places, it gets deduped and inner boundaries don't consider it + // as contributing to those boundaries. + const existingEntry = hostAsyncInfoCache.get(resource); + if (existingEntry !== undefined) { + insertSuspendedBy(existingEntry); + return; + } + + const props: { + href?: string, + media?: string, + ... + } = fiber.memoizedProps; + + // Stylesheet resources may suspend. We need to track that. + const mayResourceSuspendCommit = + resource.type === 'stylesheet' && + // If it doesn't match the currently debugged media, then it doesn't count. + (typeof props.media !== 'string' || + typeof matchMedia !== 'function' || + matchMedia(props.media)); + if (!mayResourceSuspendCommit) { + return; + } + + const instance = resource.instance; + if (instance == null) { + return; + } + + // Unlike props.href, this href will be fully qualified which we need for comparison below. + const href = instance.href; + if (typeof href !== 'string') { + return; + } + let start = -1; + let end = -1; + // $FlowFixMe[method-unbinding] + if (typeof performance.getEntriesByType === 'function') { + // We may be able to collect the start and end time of this resource from Performance Observer. + const resourceEntries = performance.getEntriesByType('resource'); + for (let i = 0; i < resourceEntries.length; i++) { + const resourceEntry = resourceEntries[i]; + if (resourceEntry.name === href) { + start = resourceEntry.startTime; + end = start + resourceEntry.duration; + } + } + } + const value = instance.sheet; + const promise = Promise.resolve(value); + (promise: any).status = 'fulfilled'; + (promise: any).value = value; + const ioInfo: ReactIOInfo = { + name: 'stylesheet', + start, + end, + value: promise, + // $FlowFixMe: This field doesn't usually take a Fiber but we're only using inside this file. + owner: fiber, // Allow linking to the if it's not filtered. + }; + const asyncInfo: ReactAsyncInfo = { + awaited: ioInfo, + // $FlowFixMe: This field doesn't usually take a Fiber but we're only using inside this file. + owner: fiber._debugOwner == null ? null : fiber._debugOwner, + debugStack: fiber._debugStack == null ? null : fiber._debugStack, + debugTask: fiber._debugTask == null ? null : fiber._debugTask, + }; + hostAsyncInfoCache.set(resource, asyncInfo); + insertSuspendedBy(asyncInfo); + } + function mountVirtualChildrenRecursively( firstChild: Fiber, lastChild: null | Fiber, // non-inclusive @@ -3446,6 +3534,7 @@ export function attach( throw new Error('Did not expect a host hoistable to be the root'); } aquireHostResource(nearestInstance, fiber.memoizedState); + trackDebugInfoFromHostResource(nearestInstance, fiber); } else if ( fiber.tag === HostComponent || fiber.tag === HostText || @@ -4282,6 +4371,7 @@ export function attach( } releaseHostResource(nearestInstance, prevFiber.memoizedState); aquireHostResource(nearestInstance, nextFiber.memoizedState); + trackDebugInfoFromHostResource(nearestInstance, nextFiber); } else if ( (nextFiber.tag === HostComponent || nextFiber.tag === HostText || 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 da74bc579e..e5e0949558 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -178,9 +178,12 @@ function SuspendedByRow({ } /> )} - {(showIOStack || !showAwaitStack) && - ioOwner !== null && - ioOwner.id !== inspectedElement.id ? ( + {ioOwner !== null && + ioOwner.id !== inspectedElement.id && + (showIOStack || + !showAwaitStack || + asyncOwner === null || + ioOwner.id !== asyncOwner.id) ? ( Date: Mon, 11 Aug 2025 20:55:48 +0200 Subject: [PATCH 17/24] Create fresh Offscreen instance when replaying (#34127) --- packages/react-reconciler/src/ReactFiber.js | 18 ----- .../src/ReactFiberBeginWork.js | 72 ++++++++++++++----- .../ReactSuspenseWithNoopRenderer-test.js | 24 +++++++ 3 files changed, 78 insertions(+), 36 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 996bc72603..ac25828400 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -24,7 +24,6 @@ import type {ActivityInstance, SuspenseInstance} from './ReactFiberConfig'; import type { LegacyHiddenProps, OffscreenProps, - OffscreenInstance, } from './ReactFiberOffscreenComponent'; import type {ViewTransitionState} from './ReactFiberViewTransitionComponent'; import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent'; @@ -76,7 +75,6 @@ import { ViewTransitionComponent, ActivityComponent, } from './ReactWorkTags'; -import {OffscreenVisible} from './ReactFiberOffscreenComponent'; import {getComponentNameFromOwner} from 'react-reconciler/src/getComponentNameFromFiber'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import { @@ -831,13 +829,6 @@ export function createFiberFromOffscreen( ): Fiber { const fiber = createFiber(OffscreenComponent, pendingProps, key, mode); fiber.lanes = lanes; - const primaryChildInstance: OffscreenInstance = { - _visibility: OffscreenVisible, - _pendingMarkers: null, - _retryCache: null, - _transitions: null, - }; - fiber.stateNode = primaryChildInstance; return fiber; } export function createFiberFromActivity( @@ -885,15 +876,6 @@ export function createFiberFromLegacyHidden( const fiber = createFiber(LegacyHiddenComponent, pendingProps, key, mode); fiber.elementType = REACT_LEGACY_HIDDEN_TYPE; fiber.lanes = lanes; - // Adding a stateNode for legacy hidden because it's currently using - // the offscreen implementation, which depends on a state node - const instance: OffscreenInstance = { - _visibility: OffscreenVisible, - _pendingMarkers: null, - _transitions: null, - _retryCache: null, - }; - fiber.stateNode = instance; return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 7a3bb4ef81..372a74f97b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -280,6 +280,7 @@ import { createCapturedValueFromError, createCapturedValueAtFiber, } from './ReactCapturedValue'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; import { createClassErrorUpdate, initializeClassErrorUpdate, @@ -620,6 +621,18 @@ function updateOffscreenComponent( const prevState: OffscreenState | null = current !== null ? current.memoizedState : null; + if (current === null && workInProgress.stateNode === null) { + // We previously reset the work-in-progress. + // We need to create a new Offscreen instance. + const primaryChildInstance: OffscreenInstance = { + _visibility: OffscreenVisible, + _pendingMarkers: null, + _retryCache: null, + _transitions: null, + }; + workInProgress.stateNode = primaryChildInstance; + } + if ( nextProps.mode === 'hidden' || (enableLegacyHidden && nextProps.mode === 'unstable-defer-without-hiding') @@ -788,6 +801,26 @@ function updateOffscreenComponent( return workInProgress.child; } +function bailoutOffscreenComponent( + current: Fiber | null, + workInProgress: Fiber, +): Fiber | null { + if ( + (current === null || current.tag !== OffscreenComponent) && + workInProgress.stateNode === null + ) { + const primaryChildInstance: OffscreenInstance = { + _visibility: OffscreenVisible, + _pendingMarkers: null, + _retryCache: null, + _transitions: null, + }; + workInProgress.stateNode = primaryChildInstance; + } + + return workInProgress.sibling; +} + function deferHiddenOffscreenComponent( current: Fiber | null, workInProgress: Fiber, @@ -1095,9 +1128,13 @@ function updateActivityComponent( if (nextProps.mode === 'hidden') { // SSR doesn't render hidden Activity so it shouldn't hydrate, // even at offscreen lane. Defer to a client rendered offscreen lane. - mountActivityChildren(workInProgress, nextProps, renderLanes); + const primaryChildFragment = mountActivityChildren( + workInProgress, + nextProps, + renderLanes, + ); workInProgress.lanes = laneToLanes(OffscreenLane); - return null; + return bailoutOffscreenComponent(null, primaryChildFragment); } else { // We must push the suspense handler context *before* attempting to // hydrate, to avoid a mismatch in case it errors. @@ -2373,7 +2410,7 @@ function updateSuspenseComponent( if (showFallback) { pushFallbackTreeSuspenseHandler(workInProgress); - const fallbackFragment = mountSuspenseFallbackChildren( + mountSuspenseFallbackChildren( workInProgress, nextPrimaryChildren, nextFallbackChildren, @@ -2408,7 +2445,7 @@ function updateSuspenseComponent( } } - return fallbackFragment; + return bailoutOffscreenComponent(null, primaryChildFragment); } else if ( enableCPUSuspense && typeof nextProps.unstable_expectedLoadTime === 'number' @@ -2417,7 +2454,7 @@ function updateSuspenseComponent( // unblock the surrounding content. Then immediately retry after the // initial commit. pushFallbackTreeSuspenseHandler(workInProgress); - const fallbackFragment = mountSuspenseFallbackChildren( + mountSuspenseFallbackChildren( workInProgress, nextPrimaryChildren, nextFallbackChildren, @@ -2444,7 +2481,7 @@ function updateSuspenseComponent( // RetryLane even if it's the one currently rendering since we're leaving // it behind on this node. workInProgress.lanes = SomeRetryLane; - return fallbackFragment; + return bailoutOffscreenComponent(null, primaryChildFragment); } else { pushPrimaryTreeSuspenseHandler(workInProgress); return mountSuspensePrimaryChildren( @@ -2479,7 +2516,7 @@ function updateSuspenseComponent( const nextFallbackChildren = nextProps.fallback; const nextPrimaryChildren = nextProps.children; - const fallbackChildFragment = updateSuspenseFallbackChildren( + updateSuspenseFallbackChildren( current, workInProgress, nextPrimaryChildren, @@ -2532,7 +2569,7 @@ function updateSuspenseComponent( renderLanes, ); workInProgress.memoizedState = SUSPENDED_MARKER; - return fallbackChildFragment; + return bailoutOffscreenComponent(current.child, primaryChildFragment); } else { if ( prevState !== null && @@ -2788,7 +2825,7 @@ function updateSuspenseFallbackChildren( primaryChildFragment.sibling = fallbackChildFragment; workInProgress.child = primaryChildFragment; - return fallbackChildFragment; + return bailoutOffscreenComponent(null, primaryChildFragment); } function retrySuspenseComponentWithoutHydrating( @@ -3094,14 +3131,13 @@ function updateDehydratedSuspenseComponent( const nextPrimaryChildren = nextProps.children; const nextFallbackChildren = nextProps.fallback; - const fallbackChildFragment = - mountSuspenseFallbackAfterRetryWithoutHydrating( - current, - workInProgress, - nextPrimaryChildren, - nextFallbackChildren, - renderLanes, - ); + mountSuspenseFallbackAfterRetryWithoutHydrating( + current, + workInProgress, + nextPrimaryChildren, + nextFallbackChildren, + renderLanes, + ); const primaryChildFragment: Fiber = (workInProgress.child: any); primaryChildFragment.memoizedState = mountSuspenseOffscreenState(renderLanes); @@ -3111,7 +3147,7 @@ function updateDehydratedSuspenseComponent( renderLanes, ); workInProgress.memoizedState = SUSPENDED_MARKER; - return fallbackChildFragment; + return bailoutOffscreenComponent(null, primaryChildFragment); } } } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 3637093529..a5c4282e9e 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -4141,4 +4141,28 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); }); + + it('can rerender after resolving a promise', async () => { + const promise = Promise.resolve(null); + const root = ReactNoop.createRoot(); + + await act(() => { + startTransition(() => { + root.render({promise}); + }); + }); + + assertLog([]); + expect(root).toMatchRenderedOutput(null); + + await act(() => { + startTransition(() => { + root.render( + +
+ , + ); + }); + }); + }); }); From de06211dbe0ce641a435b52ed3c868720aa9c633 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 12 Aug 2025 16:48:35 +0200 Subject: [PATCH 18/24] [DevTools] Send Suspense rects to frontend (#34170) --- .../profilingCommitTreeBuilder-test.js | 6 + .../src/__tests__/store-test.js | 231 +++++++++++++----- .../__tests__/storeComponentFilters-test.js | 9 + .../storeStressTestConcurrent-test.js | 98 ++++---- .../src/__tests__/treeContext-test.js | 44 +++- .../src/backend/fiber/renderer.js | 80 +++++- .../react-devtools-shared/src/constants.js | 1 + .../src/devtools/store.js | 76 +++++- .../src/devtools/utils.js | 71 +++++- .../views/Profiler/CommitTreeBuilder.js | 41 +++- .../src/frontend/types.js | 8 + packages/react-devtools-shared/src/utils.js | 55 ++++- 12 files changed, 586 insertions(+), 134 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js index f5b7e5fded..a7c0893060 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCommitTreeBuilder-test.js @@ -228,6 +228,8 @@ describe('commit tree', () => { [root] ▾ + [shell] + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` @@ -235,6 +237,8 @@ describe('commit tree', () => { ▾ + [shell] + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` @@ -299,6 +303,8 @@ describe('commit tree', () => { [root] ▾ + [shell] + `); utils.act(() => modernRender()); expect(store).toMatchInlineSnapshot(` diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 1a5a0e6a26..87524ffd04 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -24,6 +24,16 @@ describe('Store', () => { let store; let withErrorsOrWarningsIgnored; + beforeAll(() => { + // JSDDOM doesn't implement getClientRects so we're just faking one for testing purposes + Element.prototype.getClientRects = function (this: Element) { + const textContent = this.textContent; + return [ + new DOMRect(1, 2, textContent.length, textContent.split('\n').length), + ]; + }; + }); + beforeEach(() => { global.IS_REACT_ACT_ENVIRONMENT = true; @@ -123,6 +133,8 @@ describe('Store', () => { + [shell] + `); }); @@ -480,6 +492,8 @@ describe('Store', () => { + [shell] + `); await act(() => { @@ -491,6 +505,8 @@ describe('Store', () => { + [shell] + `); }); @@ -513,23 +529,31 @@ describe('Store', () => { }) => ( - }> + }> - }> + }> {suspendFirst ? ( ) : ( )} - }> + }> {suspendSecond ? ( ) : ( )} - }> + }> {suspendParent && } @@ -538,7 +562,7 @@ describe('Store', () => { ); - await act(() => + await actAsync(() => render( { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -574,15 +603,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -597,15 +631,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -620,15 +659,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -643,8 +687,13 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -659,15 +708,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -682,15 +736,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); const rendererID = getRendererID(); @@ -705,15 +764,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => agent.overrideSuspense({ @@ -726,8 +790,13 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -742,8 +811,13 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => agent.overrideSuspense({ @@ -756,15 +830,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => agent.overrideSuspense({ @@ -777,15 +856,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); await act(() => render( @@ -800,15 +884,20 @@ describe('Store', () => { [root] ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ - ▾ + ▾ + [shell] + + + + `); }); @@ -848,6 +937,8 @@ describe('Store', () => { + [shell] + `); await act(() => { @@ -861,6 +952,8 @@ describe('Store', () => { ▾ + [shell] + `); }); @@ -1197,6 +1290,8 @@ describe('Store', () => { expect(store).toMatchInlineSnapshot(` [root] ▸ + [shell] + `); // This test isn't meaningful unless we expand the suspended tree @@ -1212,6 +1307,8 @@ describe('Store', () => { + [shell] + `); await act(() => { @@ -1223,6 +1320,8 @@ describe('Store', () => { + [shell] + `); }); @@ -1447,6 +1546,8 @@ describe('Store', () => { expect(store).toMatchInlineSnapshot(` [root] ▸ + [shell] + `); await act(() => @@ -1460,6 +1561,8 @@ describe('Store', () => { ▾ + [shell] + `); const rendererID = getRendererID(); @@ -1477,6 +1580,8 @@ describe('Store', () => { ▾ + [shell] + `); await act(() => @@ -1491,6 +1596,8 @@ describe('Store', () => { ▾ + [shell] + `); }); }); @@ -1794,6 +1901,8 @@ describe('Store', () => { [root] ▾ + [shell] + `); await Promise.resolve(); @@ -1806,6 +1915,8 @@ describe('Store', () => { ▾ + [shell] + `); // Render again to unmount it @@ -2291,20 +2402,24 @@ describe('Store', () => { await actAsync(() => render()); expect(store).toMatchInlineSnapshot(` - [root] - ▾ - ▾ - - `); + [root] + ▾ + ▾ + + [shell] + + `); await actAsync(() => render()); expect(store).toMatchInlineSnapshot(` - [root] - ▾ - ▾ - - `); + [root] + ▾ + ▾ + + [shell] + + `); }); }); diff --git a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js index d7aea2981d..c29bff0538 100644 --- a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js +++ b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js @@ -156,6 +156,9 @@ describe('Store component filters', () => {
+ [shell] + + `); await actAsync( @@ -171,6 +174,9 @@ describe('Store component filters', () => {
+ [shell] + + `); await actAsync( @@ -186,6 +192,9 @@ describe('Store component filters', () => {
+ [shell] + + `); }); diff --git a/packages/react-devtools-shared/src/__tests__/storeStressTestConcurrent-test.js b/packages/react-devtools-shared/src/__tests__/storeStressTestConcurrent-test.js index 4389f78cd2..e060cb3f06 100644 --- a/packages/react-devtools-shared/src/__tests__/storeStressTestConcurrent-test.js +++ b/packages/react-devtools-shared/src/__tests__/storeStressTestConcurrent-test.js @@ -32,7 +32,7 @@ describe('StoreStressConcurrent', () => { // this helper with the real thing. actAsync = require('./utils').actAsync; - print = require('./__serializers__/storeSerializer').print; + print = require('./__serializers__/storeSerializer').printStore; }); // This is a stress test for the tree mount/update/unmount traversal. @@ -67,8 +67,7 @@ describe('StoreStressConcurrent', () => { let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); act(() => root.render({[a, b, c, d, e]})); - expect(store).toMatchInlineSnapshot( - ` + expect(store).toMatchInlineSnapshot(` [root] ▾ @@ -76,8 +75,7 @@ describe('StoreStressConcurrent', () => { - `, - ); + `); expect(container.textContent).toMatch('abcde'); const snapshotForABCDE = print(store); @@ -86,8 +84,7 @@ describe('StoreStressConcurrent', () => { act(() => { setShowX(true); }); - expect(store).toMatchInlineSnapshot( - ` + expect(store).toMatchInlineSnapshot(` [root] ▾ @@ -96,8 +93,7 @@ describe('StoreStressConcurrent', () => { - `, - ); + `); expect(container.textContent).toMatch('abxde'); const snapshotForABXDE = print(store); @@ -419,7 +415,7 @@ describe('StoreStressConcurrent', () => { ), ); // We snapshot each step once so it doesn't regress.d - snapshots.push(print(store)); + snapshots.push(print(store, false, null, false)); await act(() => root.unmount()); expect(print(store)).toBe(''); } @@ -524,7 +520,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); await act(() => root.unmount()); expect(print(store)).toBe(''); } @@ -544,7 +540,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -556,7 +552,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -567,7 +563,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -593,7 +589,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -609,7 +605,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -624,7 +620,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -646,7 +642,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -662,7 +658,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -673,7 +669,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -699,7 +695,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -711,7 +707,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -726,7 +722,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -755,7 +751,7 @@ describe('StoreStressConcurrent', () => { const suspenseID = store.getElementIDAtIndex(2); // Force fallback. - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); await actAsync(async () => { bridge.send('overrideSuspense', { id: suspenseID, @@ -763,7 +759,7 @@ describe('StoreStressConcurrent', () => { forceFallback: true, }); }); - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Stop forcing fallback. await actAsync(async () => { @@ -773,7 +769,7 @@ describe('StoreStressConcurrent', () => { forceFallback: false, }); }); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Trigger actual fallback. await act(() => @@ -789,7 +785,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Force fallback while we're in fallback mode. await act(() => { @@ -800,7 +796,7 @@ describe('StoreStressConcurrent', () => { }); }); // Keep seeing fallback content. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Switch to primary mode. await act(() => @@ -813,7 +809,7 @@ describe('StoreStressConcurrent', () => { ), ); // Fallback is still forced though. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Stop forcing fallback. This reverts to primary content. await actAsync(async () => { @@ -824,7 +820,7 @@ describe('StoreStressConcurrent', () => { }); }); // Now we see primary content. - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await actAsync(async () => root.unmount()); @@ -910,7 +906,7 @@ describe('StoreStressConcurrent', () => { ), ); // We snapshot each step once so it doesn't regress. - snapshots.push(print(store)); + snapshots.push(print(store, false, null, false)); await act(() => root.unmount()); expect(print(store)).toBe(''); } @@ -935,7 +931,7 @@ describe('StoreStressConcurrent', () => { ), ); // We snapshot each step once so it doesn't regress. - fallbackSnapshots.push(print(store)); + fallbackSnapshots.push(print(store, false, null, false)); await act(() => root.unmount()); expect(print(store)).toBe(''); } @@ -1065,7 +1061,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -1079,7 +1075,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -1092,7 +1088,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -1121,7 +1117,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(fallbackSnapshots[i]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -1140,7 +1136,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -1158,7 +1154,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(fallbackSnapshots[i]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -1182,7 +1178,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -1196,7 +1192,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -1209,7 +1205,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -1233,7 +1229,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(fallbackSnapshots[i]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[i]); // Re-render with steps[j]. await act(() => root.render( @@ -1247,7 +1243,7 @@ describe('StoreStressConcurrent', () => { ), ); // Verify the successful transition to steps[j]. - expect(print(store)).toEqual(snapshots[j]); + expect(print(store, false, null, false)).toEqual(snapshots[j]); // Check that we can transition back again. await act(() => root.render( @@ -1260,7 +1256,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(fallbackSnapshots[i]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); expect(print(store)).toBe(''); @@ -1291,7 +1287,7 @@ describe('StoreStressConcurrent', () => { const suspenseID = store.getElementIDAtIndex(2); // Force fallback. - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); await actAsync(async () => { bridge.send('overrideSuspense', { id: suspenseID, @@ -1299,7 +1295,7 @@ describe('StoreStressConcurrent', () => { forceFallback: true, }); }); - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Stop forcing fallback. await actAsync(async () => { @@ -1309,7 +1305,7 @@ describe('StoreStressConcurrent', () => { forceFallback: false, }); }); - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Trigger actual fallback. await act(() => @@ -1323,7 +1319,7 @@ describe('StoreStressConcurrent', () => { , ), ); - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Force fallback while we're in fallback mode. await act(() => { @@ -1334,7 +1330,7 @@ describe('StoreStressConcurrent', () => { }); }); // Keep seeing fallback content. - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Switch to primary mode. await act(() => @@ -1349,7 +1345,7 @@ describe('StoreStressConcurrent', () => { ), ); // Fallback is still forced though. - expect(print(store)).toEqual(fallbackSnapshots[j]); + expect(print(store, false, null, false)).toEqual(fallbackSnapshots[j]); // Stop forcing fallback. This reverts to primary content. await actAsync(async () => { @@ -1360,7 +1356,7 @@ describe('StoreStressConcurrent', () => { }); }); // Now we see primary content. - expect(print(store)).toEqual(snapshots[i]); + expect(print(store, false, null, false)).toEqual(snapshots[i]); // Clean up after every iteration. await act(() => root.unmount()); diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index fa2031c6b5..e704241805 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -1368,6 +1368,9 @@ describe('TreeListContext', () => { ▾ + [shell] + + `); const outerSuspenseID = ((store.getElementIDAtIndex(1): any): number); @@ -1407,6 +1410,9 @@ describe('TreeListContext', () => { ▾ + [shell] + + `); }); }); @@ -2361,16 +2367,20 @@ describe('TreeListContext', () => { jest.runAllTimers(); expect(state).toMatchInlineSnapshot(` - [root] - - `); + [root] + + [shell] + + `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - [root] - - `); + [root] + + [shell] + + `); }); it('should properly handle errors/warnings from components that dont mount because of Suspense', async () => { @@ -2392,9 +2402,11 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(state).toMatchInlineSnapshot(` - [root] - - `); + [root] + + [shell] + + `); await Promise.resolve(); withErrorsOrWarningsIgnored(['test-only:'], () => @@ -2414,6 +2426,8 @@ describe('TreeListContext', () => { ▾ + [shell] + `); }); @@ -2442,6 +2456,8 @@ describe('TreeListContext', () => { ▾ ✕ + [shell] + `); await Promise.resolve(); @@ -2456,10 +2472,12 @@ describe('TreeListContext', () => { ); expect(state).toMatchInlineSnapshot(` - [root] - ▾ - - `); + [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 1d4541253f..cdf5ca35b2 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -86,6 +86,7 @@ import { SUSPENSE_TREE_OPERATION_ADD, SUSPENSE_TREE_OPERATION_REMOVE, SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, + SUSPENSE_TREE_OPERATION_RESIZE, } from '../../constants'; import {inspectHooksOfFiber} from 'react-debug-tools'; import { @@ -2558,6 +2559,20 @@ export function attach( pushOperation(fiberID); pushOperation(parentID); pushOperation(nameStringID); + + const rects = suspenseInstance.rects; + if (rects === null) { + pushOperation(-1); + } else { + pushOperation(rects.length); + for (let i = 0; i < rects.length; ++i) { + const rect = rects[i]; + pushOperation(Math.round(rect.x)); + pushOperation(Math.round(rect.y)); + pushOperation(Math.round(rect.width)); + pushOperation(Math.round(rect.height)); + } + } } function recordUnmount(fiberInstance: FiberInstance): void { @@ -2606,7 +2621,30 @@ export function attach( } function recordSuspenseResize(suspenseNode: SuspenseNode): void { - // TODO: Notify the front end of the change. + if (__DEBUG__) { + console.log('recordSuspenseResize()', suspenseNode); + } + const fiberInstance = suspenseNode.instance; + if (fiberInstance.kind !== FIBER_INSTANCE) { + // TODO: Resizes of filtered Suspense nodes are currently dropped. + return; + } + + pushOperation(SUSPENSE_TREE_OPERATION_RESIZE); + pushOperation(fiberInstance.id); + const rects = suspenseNode.rects; + if (rects === null) { + pushOperation(-1); + } else { + pushOperation(rects.length); + for (let i = 0; i < rects.length; ++i) { + const rect = rects[i]; + pushOperation(Math.round(rect.x)); + pushOperation(Math.round(rect.y)); + pushOperation(Math.round(rect.width)); + pushOperation(Math.round(rect.height)); + } + } } function recordSuspenseUnmount(suspenseInstance: SuspenseNode): void { @@ -3442,7 +3480,25 @@ export function attach( // Measure this Suspense node. In general we shouldn't do this until we have // inserted the new children but since we know this is a FiberInstance we'll // just use the Fiber anyway. - newSuspenseNode.rects = measureInstance(newInstance); + // Fallbacks get attributed to the parent so we only measure if we're + // showing primary content. + if (OffscreenComponent === -1) { + const isTimedOut = fiber.memoizedState !== null; + if (!isTimedOut) { + newSuspenseNode.rects = measureInstance(newInstance); + } + } else { + const contentFiber = fiber.child; + if (contentFiber === null) { + throw new Error( + 'There should always be an Offscreen Fiber child in a Suspense boundary.', + ); + } + const isTimedOut = fiber.memoizedState !== null; + if (!isTimedOut) { + newSuspenseNode.rects = measureInstance(newInstance); + } + } recordSuspenseMount(newSuspenseNode, reconcilingParentSuspenseNode); } insertChild(newInstance); @@ -3476,7 +3532,25 @@ export function attach( // Measure this Suspense node. In general we shouldn't do this until we have // inserted the new children but since we know this is a FiberInstance we'll // just use the Fiber anyway. - newSuspenseNode.rects = measureInstance(newInstance); + // Fallbacks get attributed to the parent so we only measure if we're + // showing primary content. + if (OffscreenComponent === -1) { + const isTimedOut = fiber.memoizedState !== null; + if (!isTimedOut) { + newSuspenseNode.rects = measureInstance(newInstance); + } + } else { + const contentFiber = fiber.child; + if (contentFiber === null) { + throw new Error( + 'There should always be an Offscreen Fiber child in a Suspense boundary.', + ); + } + const isTimedOut = fiber.memoizedState !== null; + if (!isTimedOut) { + newSuspenseNode.rects = measureInstance(newInstance); + } + } } insertChild(newInstance); if (__DEBUG__) { diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index 391eea6b23..ce6ed0b308 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -27,6 +27,7 @@ export const TREE_OPERATION_SET_SUBTREE_MODE = 7; export const SUSPENSE_TREE_OPERATION_ADD = 8; export const SUSPENSE_TREE_OPERATION_REMOVE = 9; export const SUSPENSE_TREE_OPERATION_REORDER_CHILDREN = 10; +export const SUSPENSE_TREE_OPERATION_RESIZE = 11; export const PROFILING_FLAG_BASIC_SUPPORT = 0b01; export const PROFILING_FLAG_TIMELINE_SUPPORT = 0b10; diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 2d6b67ef12..f4150c7557 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -23,6 +23,7 @@ import { SUSPENSE_TREE_OPERATION_ADD, SUSPENSE_TREE_OPERATION_REMOVE, SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, + SUSPENSE_TREE_OPERATION_RESIZE, } from '../constants'; import {ElementTypeRoot} from '../frontend/types'; import { @@ -1418,6 +1419,7 @@ export default class Store extends EventEmitter<{ const id = operations[i + 1]; const parentID = operations[i + 2]; const nameStringID = operations[i + 3]; + const numRects = ((operations[i + 4]: any): number); let name = stringTable[nameStringID]; if (this._idToSuspense.has(id)) { @@ -1448,6 +1450,22 @@ export default class Store extends EventEmitter<{ } } + i += 5; + let rects: SuspenseNode['rects']; + if (numRects === -1) { + rects = null; + } else { + rects = []; + for (let rectIndex = 0; rectIndex < numRects; rectIndex++) { + const x = operations[i + 0]; + const y = operations[i + 1]; + const width = operations[i + 2]; + const height = operations[i + 3]; + rects.push({x, y, width, height}); + i += 4; + } + } + if (__DEBUG__) { debug('Suspense Add', `node ${id} as child of ${parentID}`); } @@ -1476,10 +1494,9 @@ export default class Store extends EventEmitter<{ parentID, children: [], name, + rects, }); - i += 4; - hasSuspenseTreeChanged = true; break; } @@ -1591,6 +1608,61 @@ export default class Store extends EventEmitter<{ hasSuspenseTreeChanged = true; break; } + case SUSPENSE_TREE_OPERATION_RESIZE: { + const id = ((operations[i + 1]: any): number); + const numRects = ((operations[i + 2]: any): number); + i += 3; + + const suspense = this._idToSuspense.get(id); + if (suspense === undefined) { + this._throwAndEmitError( + Error( + `Cannot set rects for suspense node "${id}" because no matching node was found in the Store.`, + ), + ); + + break; + } + + let nextRects: SuspenseNode['rects']; + if (numRects === -1) { + nextRects = null; + } else { + nextRects = []; + for (let rectIndex = 0; rectIndex < numRects; rectIndex++) { + const x = operations[i + 0]; + const y = operations[i + 1]; + const width = operations[i + 2]; + const height = operations[i + 3]; + + nextRects.push({x, y, width, height}); + + i += 4; + } + } + + suspense.rects = nextRects; + + if (__DEBUG__) { + debug( + 'Resize', + `Suspense node ${id} resize to ${ + nextRects === null + ? 'null' + : nextRects + .map( + rect => + `(${rect.x},${rect.y},${rect.width},${rect.height})`, + ) + .join(',') + }`, + ); + } + + hasSuspenseTreeChanged = true; + + break; + } default: this._throwAndEmitError( new UnsupportedBridgeOperationError( diff --git a/packages/react-devtools-shared/src/devtools/utils.js b/packages/react-devtools-shared/src/devtools/utils.js index 8ce34bf611..0501e861bb 100644 --- a/packages/react-devtools-shared/src/devtools/utils.js +++ b/packages/react-devtools-shared/src/devtools/utils.js @@ -10,7 +10,10 @@ import JSON5 from 'json5'; import type {ReactFunctionLocation} from 'shared/ReactTypes'; -import type {Element} from 'react-devtools-shared/src/frontend/types'; +import type { + Element, + SuspenseNode, +} from 'react-devtools-shared/src/frontend/types'; import type {StateContext} from './views/Components/TreeContext'; import type Store from './store'; @@ -28,6 +31,11 @@ export function printElement( key = ` key="${element.key}"`; } + let name = ''; + if (element.nameProp !== null) { + name = ` name="${element.nameProp}"`; + } + let hocDisplayNames = null; if (element.hocDisplayNames !== null) { hocDisplayNames = [...element.hocDisplayNames]; @@ -43,7 +51,45 @@ export function printElement( return `${' '.repeat(element.depth + 1)}${prefix} <${ element.displayName || 'null' - }${key}>${hocs}${suffix}`; + }${key}${name}>${hocs}${suffix}`; +} + +function printSuspense( + suspense: SuspenseNode, + includeWeight: boolean = false, +): string { + let name = ''; + if (suspense.name !== null) { + name = ` name="${suspense.name}"`; + } + + let printedRects = ''; + const rects = suspense.rects; + if (rects === null) { + printedRects = ' rects={null}'; + } else { + printedRects = ` rects={[${rects.map(rect => `{x:${rect.x},y:${rect.y},width:${rect.width},height:${rect.height}}`).join(', ')}]}`; + } + + return ``; +} + +function printSuspenseWithChildren( + store: Store, + suspense: SuspenseNode, + depth: number, +): Array { + const lines = [' '.repeat(depth) + printSuspense(suspense)]; + for (let i = 0; i < suspense.children.length; i++) { + const childID = suspense.children[i]; + const child = store.getSuspenseByID(childID); + if (child === null) { + throw new Error(`Could not find Suspense node with ID "${childID}".`); + } + lines.push(...printSuspenseWithChildren(store, child, depth + 1)); + } + + return lines; } export function printOwnersList( @@ -59,6 +105,7 @@ export function printStore( store: Store, includeWeight: boolean = false, state: StateContext | null = null, + includeSuspense: boolean = true, ): string { const snapshotLines = []; @@ -129,6 +176,26 @@ export function printStore( } rootWeight += weight; + + if (includeSuspense) { + const shell = store.getSuspenseByID(rootID); + // Roots from legacy renderers don't have a separate Suspense tree + if (shell !== null) { + if (shell.children.length > 0) { + snapshotLines.push('[shell]'); + for (let i = 0; i < shell.children.length; i++) { + const childID = shell.children[i]; + const child = store.getSuspenseByID(childID); + if (child === null) { + throw new Error( + `Could not find Suspense node with ID "${childID}".`, + ); + } + snapshotLines.push(...printSuspenseWithChildren(store, child, 1)); + } + } + } + } }); // Make sure the pretty-printed test align with the Store's reported number of total rows. diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js index d685263a22..e0bd4e7c73 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -19,6 +19,7 @@ import { SUSPENSE_TREE_OPERATION_ADD, SUSPENSE_TREE_OPERATION_REMOVE, SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, + SUSPENSE_TREE_OPERATION_RESIZE, } from 'react-devtools-shared/src/constants'; import { parseElementDisplayNameFromBackend, @@ -376,16 +377,26 @@ function updateTree( const fiberID = operations[i + 1]; const parentID = operations[i + 2]; const nameStringID = operations[i + 3]; + const numRects = operations[i + 4]; const name = stringTable[nameStringID]; - i += 4; - if (__DEBUG__) { + let rects: string; + if (numRects === -1) { + rects = 'null'; + } else { + rects = + '[' + + operations.slice(i + 5, i + 5 + numRects * 4).join(',') + + ']'; + } debug( 'Add suspense', - `node ${fiberID} (${String(name)}) under ${parentID}`, + `node ${fiberID} (name=${JSON.stringify(name)}, rects={${rects}}) under ${parentID}`, ); } + + i += 5 + (numRects === -1 ? 0 : numRects * 4); break; } @@ -416,6 +427,30 @@ function updateTree( break; } + case SUSPENSE_TREE_OPERATION_RESIZE: { + const suspenseID = ((operations[i + 1]: any): number); + const numRects = ((operations[i + 2]: any): number); + + if (__DEBUG__) { + if (numRects === -1) { + debug('Suspense resize', `suspense ${suspenseID} rects null`); + } else { + const rects = ((operations.slice( + i + 3, + i + 3 + numRects * 4, + ): any): Array); + debug( + 'Suspense resize', + `suspense ${suspenseID} rects [${rects.join(',')}]`, + ); + } + } + + i += 3 + (numRects === -1 ? 0 : numRects * 4); + + break; + } + default: throw Error(`Unsupported Bridge operation "${operation}"`); } diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 0089059df9..4c61a8b1e9 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -185,11 +185,19 @@ export type Element = { compiledWithForget: boolean, }; +export type Rect = { + x: number, + y: number, + width: number, + height: number, +}; + export type SuspenseNode = { id: Element['id'], parentID: SuspenseNode['id'] | 0, children: Array, name: string | null, + rects: null | Array, }; // Serialized version of ReactIOInfo diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index c585d90500..ea921c2988 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -43,6 +43,7 @@ import { SUSPENSE_TREE_OPERATION_ADD, SUSPENSE_TREE_OPERATION_REMOVE, SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, + SUSPENSE_TREE_OPERATION_RESIZE, } from './constants'; import { ComponentFilterElementType, @@ -339,11 +340,34 @@ export function printOperationsArray(operations: Array) { const parentID = operations[i + 2]; const nameStringID = operations[i + 3]; const name = stringTable[nameStringID]; + const numRects = operations[i + 4]; - i += 4; + i += 5; + + let rects: string; + if (numRects === -1) { + rects = 'null'; + } else { + rects = '['; + for (let rectIndex = 0; rectIndex < numRects; rectIndex++) { + const offset = i + rectIndex * 4; + const x = operations[offset + 0]; + const y = operations[offset + 1]; + const width = operations[offset + 2]; + const height = operations[offset + 3]; + + if (rectIndex > 0) { + rects += ', '; + } + rects += `(${x}, ${y}, ${width}, ${height})`; + + i += 4; + } + rects += ']'; + } logs.push( - `Add suspense node ${fiberID} (${String(name)}) under ${parentID}`, + `Add suspense node ${fiberID} (${String(name)},rects={${rects}}) under ${parentID}`, ); break; } @@ -372,6 +396,33 @@ export function printOperationsArray(operations: Array) { ); break; } + case SUSPENSE_TREE_OPERATION_RESIZE: { + const id = ((operations[i + 1]: any): number); + const numRects = ((operations[i + 2]: any): number); + i += 3; + + if (numRects === -1) { + logs.push(`Resize suspense node ${id} to null`); + } else { + let line = `Resize suspense node ${id} to [`; + for (let rectIndex = 0; rectIndex < numRects; rectIndex++) { + const x = operations[i + 0]; + const y = operations[i + 1]; + const width = operations[i + 2]; + const height = operations[i + 3]; + + if (rectIndex > 0) { + line += ', '; + } + line += `(${x}, ${y}, ${width}, ${height})`; + + i += 4; + } + logs.push(line + ']'); + } + + break; + } default: throw Error(`Unsupported Bridge operation "${operation}"`); } From 1dc3bdead16bfb6d481f29b4ec55ffcf63f5bca8 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Tue, 12 Aug 2025 11:09:35 -0400 Subject: [PATCH 19/24] Remove unused arguments from ReactElement (#34174) After various feature flag removals recently, these arguments became unused and can be deleted. --- packages/react/src/jsx/ReactJSXElement.js | 84 ++++------------------- 1 file changed, 13 insertions(+), 71 deletions(-) diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 6a562ba5e8..cb475340c9 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -156,30 +156,9 @@ function elementRefGetterWithDeprecationWarning() { * will not work. Instead test $$typeof field against Symbol.for('react.transitional.element') to check * if something is a React Element. * - * @param {*} type - * @param {*} props - * @param {*} key - * @param {string|object} ref - * @param {*} owner - * @param {*} self A *temporary* helper to detect places where `this` is - * different from the `owner` when React.createElement is called, so that we - * can warn. We want to get rid of owner and replace string `ref`s with arrow - * functions, and as long as `this` and owner are the same, there will be no - * change in behavior. - * @param {*} source An annotation object (added by a transpiler or otherwise) - * indicating filename, line number, and/or other information. * @internal */ -function ReactElement( - type, - key, - self, - source, - owner, - props, - debugStack, - debugTask, -) { +function ReactElement(type, key, props, owner, debugStack, debugTask) { // Ignore whatever was passed as the ref argument and treat `props.ref` as // the source of truth. The only thing we use this for is `element.ref`, // which will log a deprecation warning on access. In the next release, we @@ -348,16 +327,7 @@ export function jsxProd(type, config, maybeKey) { } } - return ReactElement( - type, - key, - undefined, - undefined, - getOwner(), - props, - undefined, - undefined, - ); + return ReactElement(type, key, props, getOwner(), undefined, undefined); } // While `jsxDEV` should never be called when running in production, we do @@ -376,8 +346,6 @@ export function jsxProdSignatureRunningInDevWithDynamicChildren( type, config, maybeKey, - source, - self, ) { if (__DEV__) { const isStaticChildren = false; @@ -389,8 +357,6 @@ export function jsxProdSignatureRunningInDevWithDynamicChildren( config, maybeKey, isStaticChildren, - source, - self, __DEV__ && (trackActualOwner ? Error('react-stack-top-frame') @@ -407,8 +373,6 @@ export function jsxProdSignatureRunningInDevWithStaticChildren( type, config, maybeKey, - source, - self, ) { if (__DEV__) { const isStaticChildren = true; @@ -420,8 +384,6 @@ export function jsxProdSignatureRunningInDevWithStaticChildren( config, maybeKey, isStaticChildren, - source, - self, __DEV__ && (trackActualOwner ? Error('react-stack-top-frame') @@ -442,7 +404,7 @@ const didWarnAboutKeySpread = {}; * @param {object} props * @param {string} key */ -export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { +export function jsxDEV(type, config, maybeKey, isStaticChildren) { const trackActualOwner = __DEV__ && ReactSharedInternals.recentlyCreatedOwnerStacks++ < ownerStackLimit; @@ -451,8 +413,6 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { config, maybeKey, isStaticChildren, - source, - self, __DEV__ && (trackActualOwner ? Error('react-stack-top-frame') @@ -469,8 +429,6 @@ function jsxDEVImpl( config, maybeKey, isStaticChildren, - source, - self, debugStack, debugTask, ) { @@ -491,7 +449,7 @@ function jsxDEVImpl( if (isStaticChildren) { if (isArray(children)) { for (let i = 0; i < children.length; i++) { - validateChildKeys(children[i], type); + validateChildKeys(children[i]); } if (Object.freeze) { @@ -505,7 +463,7 @@ function jsxDEVImpl( ); } } else { - validateChildKeys(children, type); + validateChildKeys(children); } } @@ -591,16 +549,7 @@ function jsxDEVImpl( defineKeyPropWarningGetter(props, displayName); } - return ReactElement( - type, - key, - self, - source, - getOwner(), - props, - debugStack, - debugTask, - ); + return ReactElement(type, key, props, getOwner(), debugStack, debugTask); } } @@ -620,7 +569,7 @@ export function createElement(type, config, children) { // prod. (Rendering will throw with a helpful message and as soon as the // type is fixed, the key warnings will appear.) for (let i = 2; i < arguments.length; i++) { - validateChildKeys(arguments[i], type); + validateChildKeys(arguments[i]); } // Unlike the jsx() runtime, createElement() doesn't warn about key spread. @@ -721,10 +670,8 @@ export function createElement(type, config, children) { return ReactElement( type, key, - undefined, - undefined, - getOwner(), props, + getOwner(), __DEV__ && (trackActualOwner ? Error('react-stack-top-frame') @@ -740,10 +687,8 @@ export function cloneAndReplaceKey(oldElement, newKey) { const clonedElement = ReactElement( oldElement.type, newKey, - undefined, - undefined, - !__DEV__ ? undefined : oldElement._owner, oldElement.props, + !__DEV__ ? undefined : oldElement._owner, __DEV__ && oldElement._debugStack, __DEV__ && oldElement._debugTask, ); @@ -829,16 +774,14 @@ export function cloneElement(element, config, children) { const clonedElement = ReactElement( element.type, key, - undefined, - undefined, - owner, props, + owner, __DEV__ && element._debugStack, __DEV__ && element._debugTask, ); for (let i = 2; i < arguments.length; i++) { - validateChildKeys(arguments[i], clonedElement.type); + validateChildKeys(arguments[i]); } return clonedElement; @@ -853,10 +796,9 @@ export function cloneElement(element, config, children) { * @param {ReactNode} node Statically passed child of any type. * @param {*} parentType node's parent's type. */ -function validateChildKeys(node, parentType) { +function validateChildKeys(node) { if (__DEV__) { - // With owner stacks is, no warnings happens. All we do is - // mark elements as being in a valid static child position so they + // Mark elements as being in a valid static child position so they // don't need keys. if (isValidElement(node)) { if (node._store) { From 47fd2f5e1487fb48c561a6f8f30c534d8f8c7747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 12 Aug 2025 13:57:35 -0400 Subject: [PATCH 20/24] [DevTools] Fix index (#34187) I used the wrong indexer and tested with one entry. --- packages/react-devtools-shared/src/backend/fiber/renderer.js | 2 +- 1 file changed, 1 insertion(+), 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 cdf5ca35b2..f5d202fe01 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -3247,7 +3247,7 @@ export function attach( const debugInfo = thenable._debugInfo; if (debugInfo) { for (let j = 0; j < debugInfo.length; j++) { - const debugEntry = debugInfo[i]; + const debugEntry = debugInfo[j]; if (debugEntry.awaited) { const asyncInfo: ReactAsyncInfo = (debugEntry: any); insertSuspendedBy(asyncInfo); From 0422a00e3e5e6823ae763bef37911ee50709c324 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 12 Aug 2025 19:58:19 +0200 Subject: [PATCH 21/24] [DevTools] Fix missing key warning (#34186) --- .../src/devtools/views/Components/InspectedElementView.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js index 1318e96c30..d7fd2c9ae5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js @@ -174,9 +174,8 @@ export default function InspectedElementView({ {showStack ? : null} {showOwnersList && owners?.map(owner => ( - <> + 0 ? ( ) : null} - + ))} {rootType !== null && ( From 9baecbf02b2b53746f259b77293288aa9d8968f8 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 12 Aug 2025 16:46:56 -0700 Subject: [PATCH 22/24] [Fizz] Avoid hanging when suspending after aborting while rendering (#34192) This fixes an edge case where you abort the render while rendering a component that ends up Suspending. It technically only applied if you were deep enough to be inside `renderNode` and was not susceptible to hanging if the abort + suspending component was being tried inside retryRenderTask/retryReplaytask. The fix is to preempt the thenable checks in renderNode and check if the request is aborting and if so just bubble up to the task handler. The reason this hung before is a new task would get scheduled after we had aborted every other task (minus the currently rendering one). This led to a situation where the task count would not hit zero. --- .../src/__tests__/ReactDOMFizzServer-test.js | 54 +++++++++++++++++-- packages/react-server/src/ReactFizzServer.js | 8 ++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 22d2279578..8442fa3c13 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -94,9 +94,7 @@ describe('ReactDOMFizzServer', () => { ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMFizzServer = require('react-dom/server'); - if (__EXPERIMENTAL__) { - ReactDOMFizzStatic = require('react-dom/static'); - } + ReactDOMFizzStatic = require('react-dom/static'); Stream = require('stream'); Suspense = React.Suspense; use = React.use; @@ -10784,4 +10782,54 @@ Unfortunately that previous paragraph wasn't quite long enough so I'll continue // Instead we assert that we never emitted the fallback of the Suspense boundary around the body. expect(streamedContent).not.toContain(randomTag); }); + + it('should be able to Suspend after aborting in the same component without hanging the render', async () => { + const controller = new AbortController(); + + const promise1 = new Promise(() => {}); + function AbortAndSuspend() { + controller.abort('boom'); + return React.use(promise1); + } + + function App() { + return ( + + + + {/* + The particular code path that was problematic required the Suspend to happen in renderNode + rather than retryRenderTask so we render the aborting function inside a host component + intentionally here + */} +
+ +
+
+ + + ); + } + + const errors = []; + await act(async () => { + const result = await ReactDOMFizzStatic.prerenderToNodeStream(, { + signal: controller.signal, + onError(e) { + errors.push(e); + }, + }); + + result.prelude.pipe(writable); + }); + + expect(errors).toEqual(['boom']); + + expect(getVisibleChildren(document)).toEqual( + + + loading... + , + ); + }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index d619385ec7..8681d03b22 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -4155,7 +4155,9 @@ function renderNode( getSuspendedThenable() : thrownValue; - if (typeof x === 'object' && x !== null) { + if (request.status === ABORTING) { + // We are aborting so we can just bubble up to the task by falling through + } else if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { const wakeable: Wakeable = (x: any); @@ -4254,7 +4256,9 @@ function renderNode( getSuspendedThenable() : thrownValue; - if (typeof x === 'object' && x !== null) { + if (request.status === ABORTING) { + // We are aborting so we can just bubble up to the task by falling through + } else if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] if (typeof x.then === 'function') { const wakeable: Wakeable = (x: any); From cbea070ac9507e0d3259513ae4b504550ac87751 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 13 Aug 2025 09:28:42 +0900 Subject: [PATCH 23/24] [compiler] new tests for props derived Adds some new test cases for ValidateNoDerivedComputationsInEffects. --- ...ved-state-one-time-init-no-error.expect.md | 87 +++++++++++++++++++ .../derived-state-one-time-init-no-error.js | 21 +++++ ...-state-with-conditional-no-error.expect.md | 79 +++++++++++++++++ ...derived-state-with-conditional-no-error.js | 21 +++++ ...state-with-side-effects-no-error.expect.md | 74 ++++++++++++++++ ...erived-state-with-side-effects-no-error.js | 19 ++++ ...ug-derived-state-from-mixed-deps.expect.md | 49 +++++++++++ ...error.bug-derived-state-from-mixed-deps.js | 23 +++++ ...ed-state-from-props-destructured.expect.md | 43 +++++++++ ...d-derived-state-from-props-destructured.js | 17 ++++ ...rived-state-from-props-in-effect.expect.md | 43 +++++++++ ...alid-derived-state-from-props-in-effect.js | 17 ++++ ...rived-state-from-state-in-effect.expect.md | 51 +++++++++++ ...alid-derived-state-from-state-in-effect.js | 25 ++++++ ...erived-state-from-props-computed.expect.md | 72 +++++++++++++++ ...valid-derived-state-from-props-computed.js | 18 ++++ 16 files changed, 659 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.expect.md new file mode 100644 index 0000000000..07a58aeef3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.expect.md @@ -0,0 +1,87 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({initialName}) { + const [name, setName] = useState(''); + + useEffect(() => { + setName(initialName); + }, []); + + return ( +
+ setName(e.target.value)} /> +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{initialName: 'John'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(6); + const { initialName } = t0; + const [name, setName] = useState(""); + let t1; + if ($[0] !== initialName) { + t1 = () => { + setName(initialName); + }; + $[0] = initialName; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = []; + $[2] = t2; + } else { + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t3 = (e) => setName(e.target.value); + $[3] = t3; + } else { + t3 = $[3]; + } + let t4; + if ($[4] !== name) { + t4 = ( +
+ +
+ ); + $[4] = name; + $[5] = t4; + } else { + t4 = $[5]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ initialName: "John" }], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.js new file mode 100644 index 0000000000..c6705378a5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-one-time-init-no-error.js @@ -0,0 +1,21 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({initialName}) { + const [name, setName] = useState(''); + + useEffect(() => { + setName(initialName); + }, []); + + return ( +
+ setName(e.target.value)} /> +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{initialName: 'John'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.expect.md new file mode 100644 index 0000000000..b7a1c85d52 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.expect.md @@ -0,0 +1,79 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({value, enabled}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + if (enabled) { + setLocalValue(value); + } else { + setLocalValue('disabled'); + } + }, [value, enabled]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test', enabled: true}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(6); + const { value, enabled } = t0; + const [localValue, setLocalValue] = useState(""); + let t1; + let t2; + if ($[0] !== enabled || $[1] !== value) { + t1 = () => { + if (enabled) { + setLocalValue(value); + } else { + setLocalValue("disabled"); + } + }; + + t2 = [value, enabled]; + $[0] = enabled; + $[1] = value; + $[2] = t1; + $[3] = t2; + } else { + t1 = $[2]; + t2 = $[3]; + } + useEffect(t1, t2); + let t3; + if ($[4] !== localValue) { + t3 =
{localValue}
; + $[4] = localValue; + $[5] = t3; + } else { + t3 = $[5]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: "test", enabled: true }], +}; + +``` + +### Eval output +(kind: ok)
test
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.js new file mode 100644 index 0000000000..79d83b8925 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.js @@ -0,0 +1,21 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({value, enabled}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + if (enabled) { + setLocalValue(value); + } else { + setLocalValue('disabled'); + } + }, [value, enabled]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test', enabled: true}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.expect.md new file mode 100644 index 0000000000..e0708dd1f7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.expect.md @@ -0,0 +1,74 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({value}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + console.log('Value changed:', value); + setLocalValue(value); + document.title = `Value: ${value}`; + }, [value]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(5); + const { value } = t0; + const [localValue, setLocalValue] = useState(""); + let t1; + let t2; + if ($[0] !== value) { + t1 = () => { + console.log("Value changed:", value); + setLocalValue(value); + document.title = `Value: ${value}`; + }; + t2 = [value]; + $[0] = value; + $[1] = t1; + $[2] = t2; + } else { + t1 = $[1]; + t2 = $[2]; + } + useEffect(t1, t2); + let t3; + if ($[3] !== localValue) { + t3 =
{localValue}
; + $[3] = localValue; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: "test" }], +}; + +``` + +### Eval output +(kind: ok)
test
+logs: ['Value changed:','test'] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.js new file mode 100644 index 0000000000..b948dda6cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.js @@ -0,0 +1,19 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({value}) { + const [localValue, setLocalValue] = useState(''); + + useEffect(() => { + console.log('Value changed:', value); + setLocalValue(value); + document.title = `Value: ${value}`; + }, [value]); + + return
{localValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 'test'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md new file mode 100644 index 0000000000..54c95d68e3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({prefix}) { + const [name, setName] = useState(''); + const [displayName, setDisplayName] = useState(''); + + useEffect(() => { + setDisplayName(prefix + name); + }, [prefix, name]); + + return ( +
+ setName(e.target.value)} /> +
{displayName}
+
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prefix: 'Hello, '}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.derived-state-from-mixed-deps-no-error.ts:9:4 + 7 | + 8 | useEffect(() => { +> 9 | setDisplayName(prefix + name); + | ^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 10 | }, [prefix, name]); + 11 | + 12 | return ( +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.js new file mode 100644 index 0000000000..0004ab0ebf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.js @@ -0,0 +1,23 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({prefix}) { + const [name, setName] = useState(''); + const [displayName, setDisplayName] = useState(''); + + useEffect(() => { + setDisplayName(prefix + name); + }, [prefix, name]); + + return ( +
+ setName(e.target.value)} /> +
{displayName}
+
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prefix: 'Hello, '}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md new file mode 100644 index 0000000000..cb18bd12a3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({user: {firstName, lastName}}) { + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {firstName: 'John', lastName: 'Doe'}}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-state-from-props-destructured.ts:8:4 + 6 | + 7 | useEffect(() => { +> 8 | setFullName(firstName + ' ' + lastName); + | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 9 | }, [firstName, lastName]); + 10 | + 11 | return
{fullName}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js new file mode 100644 index 0000000000..130d31c11a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js @@ -0,0 +1,17 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({user: {firstName, lastName}}) { + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{user: {firstName: 'John', lastName: 'Doe'}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md new file mode 100644 index 0000000000..15d94c39ad --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({firstName, lastName}) { + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstName: 'John', lastName: 'Doe'}], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-state-from-props-in-effect.ts:8:4 + 6 | + 7 | useEffect(() => { +> 8 | setFullName(firstName + ' ' + lastName); + | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 9 | }, [firstName, lastName]); + 10 | + 11 | return
{fullName}
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.js new file mode 100644 index 0000000000..966f09ea89 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.js @@ -0,0 +1,17 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component({firstName, lastName}) { + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return
{fullName}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{firstName: 'John', lastName: 'Doe'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md new file mode 100644 index 0000000000..7466edb3c5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md @@ -0,0 +1,51 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component() { + const [firstName, setFirstName] = useState('John'); + const [lastName, setLastName] = useState('Doe'); + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return ( +
+ setFirstName(e.target.value)} /> + setLastName(e.target.value)} /> +
{fullName}
+
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + + +## Error + +``` +Found 1 error: + +Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +error.invalid-derived-state-from-state-in-effect.ts:10:4 + 8 | + 9 | useEffect(() => { +> 10 | setFullName(firstName + ' ' + lastName); + | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + 11 | }, [firstName, lastName]); + 12 | + 13 | return ( +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.js new file mode 100644 index 0000000000..2b4f9f7066 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.js @@ -0,0 +1,25 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component() { + const [firstName, setFirstName] = useState('John'); + const [lastName, setLastName] = useState('Doe'); + const [fullName, setFullName] = useState(''); + + useEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + + return ( +
+ setFirstName(e.target.value)} /> + setLastName(e.target.value)} /> +
{fullName}
+
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.expect.md new file mode 100644 index 0000000000..3d0c4fe9c8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.expect.md @@ -0,0 +1,72 @@ + +## Input + +```javascript +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component(props) { + const [displayValue, setDisplayValue] = useState(''); + + useEffect(() => { + const computed = props.prefix + props.value + props.suffix; + setDisplayValue(computed); + }, [props.prefix, props.value, props.suffix]); + + return
{displayValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prefix: '[', value: 'test', suffix: ']'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects +import { useEffect, useState } from "react"; + +function Component(props) { + const $ = _c(7); + const [displayValue, setDisplayValue] = useState(""); + let t0; + let t1; + if ($[0] !== props.prefix || $[1] !== props.suffix || $[2] !== props.value) { + t0 = () => { + const computed = props.prefix + props.value + props.suffix; + setDisplayValue(computed); + }; + t1 = [props.prefix, props.value, props.suffix]; + $[0] = props.prefix; + $[1] = props.suffix; + $[2] = props.value; + $[3] = t0; + $[4] = t1; + } else { + t0 = $[3]; + t1 = $[4]; + } + useEffect(t0, t1); + let t2; + if ($[5] !== displayValue) { + t2 =
{displayValue}
; + $[5] = displayValue; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ prefix: "[", value: "test", suffix: "]" }], +}; + +``` + +### Eval output +(kind: ok)
[test]
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.js new file mode 100644 index 0000000000..0e726f86ab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.js @@ -0,0 +1,18 @@ +// @validateNoDerivedComputationsInEffects +import {useEffect, useState} from 'react'; + +function Component(props) { + const [displayValue, setDisplayValue] = useState(''); + + useEffect(() => { + const computed = props.prefix + props.value + props.suffix; + setDisplayValue(computed); + }, [props.prefix, props.value, props.suffix]); + + return
{displayValue}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prefix: '[', value: 'test', suffix: ']'}], +}; From 38e718ed68dbf356c53000887935b577f07b1aea Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 13 Aug 2025 09:28:42 +0900 Subject: [PATCH 24/24] [compiler][wip] Extend ValidateNoDerivedComputationsInEffects for props derived effects This PR adds infra to disambiguate between two types of derived state in effects: 1. State derived from props 2. State derived from other state TODO: - [ ] Props tracking through destructuring and property access does not seem to be propagated correctly inside of Functions' instructions (or i might be misunderstanding how we track aliasing effects) - [ ] compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/invalid-derived-state-from-props-computed.js should be failing - [ ] Handle "mixed" case where deps flow from at least one prop AND state. Should probably have a different error reason, to aid with categorization --- .../ValidateNoDerivedComputationsInEffects.ts | 184 ++++++++++++++++-- ...id-derived-computation-in-effect.expect.md | 6 +- ...ug-derived-state-from-mixed-deps.expect.md | 8 +- ...ed-state-from-props-destructured.expect.md | 6 +- ...d-derived-state-from-props-destructured.js | 4 +- ...rived-state-from-props-in-effect.expect.md | 6 +- ...rived-state-from-state-in-effect.expect.md | 6 +- 7 files changed, 194 insertions(+), 26 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts index d026a94ed4..d45e18c448 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects.ts @@ -12,14 +12,21 @@ import { FunctionExpression, HIRFunction, IdentifierId, + Place, isSetStateType, isUseEffectHookType, } from '../HIR'; +import {printInstruction, printPlace} from '../HIR/PrintHIR'; import { eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; +type SetStateCall = { + loc: SourceLocation; + propsSource: Place | null; // null means state-derived, non-null means props-derived +}; + /** * Validates that useEffect is not used for derived computations which could/should * be performed in render. @@ -47,12 +54,96 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void { const candidateDependencies: Map = new Map(); const functions: Map = new Map(); const locals: Map = new Map(); + const derivedFromProps: Map = new Map(); const errors = new CompilerError(); + if (fn.fnType === 'Hook') { + for (const param of fn.params) { + if (param.kind === 'Identifier') { + derivedFromProps.set(param.identifier.id, param); + } + } + } else if (fn.fnType === 'Component') { + const props = fn.params[0]; + if (props != null && props.kind === 'Identifier') { + derivedFromProps.set(props.identifier.id, props); + } + } + for (const block of fn.body.blocks.values()) { for (const instr of block.instructions) { const {lvalue, value} = instr; + + // Track props derivation through instruction effects + if (instr.effects != null) { + for (const effect of instr.effects) { + switch (effect.kind) { + case 'Assign': + case 'Alias': + case 'MaybeAlias': + case 'Capture': { + const source = derivedFromProps.get(effect.from.identifier.id); + if (source != null) { + derivedFromProps.set(effect.into.identifier.id, source); + } + break; + } + } + } + } + + /** + * TODO: figure out why property access off of props does not create an Assign or Alias/Maybe + * Alias + * + * import {useEffect, useState} from 'react' + * + * function Component(props) { + * const [displayValue, setDisplayValue] = useState(''); + * + * useEffect(() => { + * const computed = props.prefix + props.value + props.suffix; + * ^^^^^^^^^^^^ ^^^^^^^^^^^ ^^^^^^^^^^^^ + * we want to track that these are from props + * setDisplayValue(computed); + * }, [props.prefix, props.value, props.suffix]); + * + * return
{displayValue}
; + * } + */ + if (value.kind === 'FunctionExpression') { + for (const [, block] of value.loweredFunc.func.body.blocks) { + for (const instr of block.instructions) { + if (instr.effects != null) { + console.group(printInstruction(instr)); + for (const effect of instr.effects) { + console.log(effect); + switch (effect.kind) { + case 'Assign': + case 'Alias': + case 'MaybeAlias': + case 'Capture': { + const source = derivedFromProps.get( + effect.from.identifier.id, + ); + if (source != null) { + derivedFromProps.set(effect.into.identifier.id, source); + } + break; + } + } + } + } + console.groupEnd(); + } + } + } + + for (const [, place] of derivedFromProps) { + console.log(printPlace(place)); + } + if (value.kind === 'LoadLocal') { locals.set(lvalue.identifier.id, value.place.identifier.id); } else if (value.kind === 'ArrayExpression') { @@ -89,6 +180,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void { validateEffect( effectFunction.loweredFunc.func, dependencies, + derivedFromProps, errors, ); } @@ -104,6 +196,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void { function validateEffect( effectFunction: HIRFunction, effectDeps: Array, + derivedFromProps: Map, errors: CompilerError, ): void { for (const operand of effectFunction.context) { @@ -111,16 +204,22 @@ function validateEffect( continue; } else if (effectDeps.find(dep => dep === operand.identifier.id) != null) { continue; + } else if (derivedFromProps.has(operand.identifier.id)) { + continue; } else { // Captured something other than the effect dep or setState + console.log('early return 1'); return; } } for (const dep of effectDeps) { + console.log({dep}); if ( effectFunction.context.find(operand => operand.identifier.id === dep) == - null + null || + derivedFromProps.has(dep) === false ) { + console.log('early return 2'); // effect dep wasn't actually used in the function return; } @@ -128,11 +227,18 @@ function validateEffect( const seenBlocks: Set = new Set(); const values: Map> = new Map(); + const effectDerivedFromProps: Map = new Map(); + for (const dep of effectDeps) { + console.log({dep}); values.set(dep, [dep]); + const propsSource = derivedFromProps.get(dep); + if (propsSource != null) { + effectDerivedFromProps.set(dep, propsSource); + } } - const setStateLocations: Array = []; + const setStateCalls: Array = []; for (const block of effectFunction.body.blocks.values()) { for (const pred of block.preds) { if (!seenBlocks.has(pred)) { @@ -142,6 +248,8 @@ function validateEffect( } for (const phi of block.phis) { const aggregateDeps: Set = new Set(); + let propsSource: Place | null = null; + for (const operand of phi.operands.values()) { const deps = values.get(operand.identifier.id); if (deps != null) { @@ -149,10 +257,18 @@ function validateEffect( aggregateDeps.add(dep); } } + const source = effectDerivedFromProps.get(operand.identifier.id); + if (source != null) { + propsSource = source; + } } + if (aggregateDeps.size !== 0) { values.set(phi.place.identifier.id, Array.from(aggregateDeps)); } + if (propsSource != null) { + effectDerivedFromProps.set(phi.place.identifier.id, propsSource); + } } for (const instr of block.instructions) { switch (instr.value.kind) { @@ -195,9 +311,16 @@ function validateEffect( ) { const deps = values.get(instr.value.args[0].identifier.id); if (deps != null && new Set(deps).size === effectDeps.length) { - setStateLocations.push(instr.value.callee.loc); + const propsSource = effectDerivedFromProps.get( + instr.value.args[0].identifier.id, + ); + + setStateCalls.push({ + loc: instr.value.callee.loc, + propsSource: propsSource ?? null, + }); } else { - // doesn't depend on any deps + // doesn't depend on all deps return; } } @@ -207,6 +330,26 @@ function validateEffect( return; } } + + // Track props derivation through instruction effects + if (instr.effects != null) { + for (const effect of instr.effects) { + switch (effect.kind) { + case 'Assign': + case 'Alias': + case 'MaybeAlias': + case 'Capture': { + const source = effectDerivedFromProps.get( + effect.from.identifier.id, + ); + if (source != null) { + effectDerivedFromProps.set(effect.into.identifier.id, source); + } + break; + } + } + } + } } for (const operand of eachTerminalOperand(block.terminal)) { if (values.has(operand.identifier.id)) { @@ -217,14 +360,29 @@ function validateEffect( seenBlocks.add(block.id); } - for (const loc of setStateLocations) { - errors.push({ - reason: - 'Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)', - description: null, - severity: ErrorSeverity.InvalidReact, - loc, - suggestions: null, - }); + for (const call of setStateCalls) { + if (call.propsSource != null) { + const propName = call.propsSource.identifier.name?.value; + const propInfo = propName != null ? ` (from prop '${propName}')` : ''; + + errors.push({ + reason: `Consider lifting state up to the parent component to make this a controlled component. (https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes)`, + description: `You are using props${propInfo} to update local state in an effect.`, + severity: ErrorSeverity.InvalidReact, + loc: call.loc, + suggestions: null, + }); + } else { + errors.push({ + reason: + 'You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)', + description: + 'This effect updates state based on other state values. ' + + 'Consider calculating this value directly during render', + severity: ErrorSeverity.InvalidReact, + loc: call.loc, + suggestions: null, + }); + } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md index d97a665ae6..1d7e24b3ef 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-derived-computation-in-effect.expect.md @@ -24,13 +24,15 @@ function BadExample() { ``` Found 1 error: -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) +Error: You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +This effect updates state based on other state values. Consider calculating this value directly during render. error.invalid-derived-computation-in-effect.ts:9:4 7 | const [fullName, setFullName] = useState(''); 8 | useEffect(() => { > 9 | setFullName(capitalize(firstName + ' ' + lastName)); - | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^ You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 10 | }, [firstName, lastName]); 11 | 12 | return
{fullName}
; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md index 54c95d68e3..8124f4b3f3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md @@ -34,13 +34,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) +Error: You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) -error.derived-state-from-mixed-deps-no-error.ts:9:4 +This effect updates state based on other state values. Consider calculating this value directly during render. + +error.bug-derived-state-from-mixed-deps.ts:9:4 7 | 8 | useEffect(() => { > 9 | setDisplayName(prefix + name); - | ^^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^^^^ You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 10 | }, [prefix, name]); 11 | 12 | return ( diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md index cb18bd12a3..26b8b7930b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.expect.md @@ -28,13 +28,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) +Error: You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +This effect updates state based on other state values. Consider calculating this value directly during render. error.invalid-derived-state-from-props-destructured.ts:8:4 6 | 7 | useEffect(() => { > 8 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^ You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 9 | }, [firstName, lastName]); 10 | 11 | return
{fullName}
; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js index 130d31c11a..966f09ea89 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-destructured.js @@ -1,7 +1,7 @@ // @validateNoDerivedComputationsInEffects import {useEffect, useState} from 'react'; -function Component({user: {firstName, lastName}}) { +function Component({firstName, lastName}) { const [fullName, setFullName] = useState(''); useEffect(() => { @@ -13,5 +13,5 @@ function Component({user: {firstName, lastName}}) { export const FIXTURE_ENTRYPOINT = { fn: Component, - params: [{user: {firstName: 'John', lastName: 'Doe'}}], + params: [{firstName: 'John', lastName: 'Doe'}], }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md index 15d94c39ad..1f7ff8dc5d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-props-in-effect.expect.md @@ -28,13 +28,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) +Error: You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +This effect updates state based on other state values. Consider calculating this value directly during render. error.invalid-derived-state-from-props-in-effect.ts:8:4 6 | 7 | useEffect(() => { > 8 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^ You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 9 | }, [firstName, lastName]); 10 | 11 | return
{fullName}
; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md index 7466edb3c5..c5548c970b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.invalid-derived-state-from-state-in-effect.expect.md @@ -36,13 +36,15 @@ export const FIXTURE_ENTRYPOINT = { ``` Found 1 error: -Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) +Error: You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + +This effect updates state based on other state values. Consider calculating this value directly during render. error.invalid-derived-state-from-state-in-effect.ts:10:4 8 | 9 | useEffect(() => { > 10 | setFullName(firstName + ' ' + lastName); - | ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) + | ^^^^^^^^^^^ You may not need this effect. Values derived from state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state) 11 | }, [firstName, lastName]); 12 | 13 | return (