From 5bbf9be2468ee80ee4558d4afa8570d8582866bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 31 Jul 2025 10:30:10 -0400 Subject: [PATCH 1/2] [DevTools] Model Hidden Offscreen Boundaries as Unmounts (#34062) This is modeling Offscreen boundaries as the thing that unmounts a tree in the frontend. This will let us model this as a "hide" that preserves state instead in a follow up but not yet. By doing it this way, we don't have to special case suspended Suspense boundaries, at least not for the modern versions that use Offscreen as the internal node. It's still special cased for the old React versions. Instead, this is handled by the Offscreen fiber getting hidden. By giving this fiber an FilteredFiberInstance, we also have somewhere to store the children on (separately from the parent children set which can include other siblings too like the loading state). One consequence is that Activity boundary content now disappears when they're hidden which is probably a good thing since otherwise it would be confusing and noisy when it's used to render multiple pages at once. --- .../__tests__/storeComponentFilters-test.js | 65 ++++--- .../src/backend/fiber/renderer.js | 173 +++++++++++------- 2 files changed, 142 insertions(+), 96 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js index c0ab51a1718..d7aea2981d8 100644 --- a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js +++ b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js @@ -207,12 +207,11 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- ▾ -
- `); + [root] + ▾ +
+ + `); await actAsync( async () => @@ -222,10 +221,9 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] -
-
- `); + [root] +
+ `); await actAsync( async () => @@ -235,12 +233,11 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- ▾ -
- `); + [root] + ▾ +
+ + `); } }); @@ -262,12 +259,12 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- ▾ -
- `); + [root] + ▾ +
+ ▾ +
+ `); await actAsync( async () => @@ -277,12 +274,12 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- ▾ -
- `); + [root] + ▾ +
+ ▾ +
+ `); await actAsync( async () => @@ -292,12 +289,12 @@ describe('Store component filters', () => { ); expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- ▾ -
- `); + [root] + ▾ +
+ ▾ +
+ `); } }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 7dac18289e8..11346e7ec31 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -205,7 +205,7 @@ type FilteredFiberInstance = { source: null | string | Error | ReactFunctionLocation, // always null here. logCount: number, // total number of errors/warnings last seen treeBaseDuration: number, // the profiled time of the last render of this subtree - suspendedBy: null | Array, // not used + suspendedBy: null | Array, // only used at the root suspenseNode: null | SuspenseNode, data: Fiber, // one of a Fiber pair }; @@ -2236,13 +2236,17 @@ export function attach( // the debugStack will be a stack frame inside the ownerInstance's source. ownerInstance.source = fiber._debugStack; } + + let unfilteredParent = parentInstance; + while ( + unfilteredParent !== null && + unfilteredParent.kind === FILTERED_FIBER_INSTANCE + ) { + unfilteredParent = unfilteredParent.parent; + } + const ownerID = ownerInstance === null ? 0 : ownerInstance.id; - const parentID = parentInstance - ? parentInstance.kind === FILTERED_FIBER_INSTANCE - ? // A Filtered Fiber Instance will always have a Virtual Instance as a parent. - ((parentInstance.parent: any): VirtualInstance).id - : parentInstance.id - : 0; + const parentID = unfilteredParent === null ? 0 : unfilteredParent.id; const displayNameStringID = getStringID(displayName); @@ -2331,13 +2335,17 @@ export function attach( // the debugStack will be a stack frame inside the ownerInstance's source. ownerInstance.source = componentInfo.debugStack; } + + let unfilteredParent = parentInstance; + while ( + unfilteredParent !== null && + unfilteredParent.kind === FILTERED_FIBER_INSTANCE + ) { + unfilteredParent = unfilteredParent.parent; + } + const ownerID = ownerInstance === null ? 0 : ownerInstance.id; - const parentID = parentInstance - ? parentInstance.kind === FILTERED_FIBER_INSTANCE - ? // A Filtered Fiber Instance will always have a Virtual Instance as a parent. - ((parentInstance.parent: any): VirtualInstance).id - : parentInstance.id - : 0; + const parentID = unfilteredParent === null ? 0 : unfilteredParent.id; const displayNameStringID = getStringID(displayName); @@ -2436,13 +2444,22 @@ export function attach( // we bubble that up to the nearest parent Suspense boundary that isn't in fallback mode. parentSuspenseNode = parentSuspenseNode.parent; } - const parentInstance = reconcilingParent; - if (parentInstance === null || parentSuspenseNode === null) { + if (reconcilingParent === null || parentSuspenseNode === null) { throw new Error( 'It should not be possible to have suspended data outside the root. ' + 'Even suspending at the first position is still a child of the root.', ); } + // Use the nearest unfiltered parent so that there's always some component that has + // the entry on it even if you filter, or the root if all are filtered. + let parentInstance = reconcilingParent; + while ( + parentInstance.kind === FILTERED_FIBER_INSTANCE && + parentInstance.parent !== null + ) { + parentInstance = parentInstance.parent; + } + const suspenseNodeSuspendedBy = parentSuspenseNode.suspendedBy; const ioInfo = asyncInfo.awaited; let suspendedBySet = suspenseNodeSuspendedBy.get(ioInfo); @@ -2778,7 +2795,7 @@ export function attach( // so we assume insertSuspendedBy dedupes. insertSuspendedBy(asyncInfo); } - if (previousVirtualInstance) continue; + continue; } if (typeof debugEntry.name !== 'string') { // Not a Component. Some other Debug Info. @@ -2893,7 +2910,8 @@ export function attach( } else if ( (reconcilingParent !== null && reconcilingParent.kind === VIRTUAL_INSTANCE) || - fiber.tag === SuspenseComponent + fiber.tag === SuspenseComponent || + fiber.tag === OffscreenComponent // Use to keep resuspended instances alive inside a SuspenseComponent. ) { // If the parent is a Virtual Instance and we filtered this Fiber we include a // hidden node. We also include this if it's a Suspense boundary so we can track those @@ -2977,7 +2995,11 @@ export function attach( aquireHostInstance(nearestInstance, fiber.stateNode); } - if (fiber.tag === SuspenseComponent) { + if (fiber.tag === OffscreenComponent && fiber.memoizedState !== null) { + // If an Offscreen component is hidden, don't mount its children yet. + } else if (fiber.tag === SuspenseComponent && OffscreenComponent === -1) { + // Legacy Suspense without the Offscreen wrapper. For the modern Suspense we just handle the + // Offscreen wrapper itself specially. const isTimedOut = fiber.memoizedState !== null; if (isTimedOut) { // Special case: if Suspense mounts in a timed-out state, @@ -2999,15 +3021,7 @@ export function attach( } // TODO: Track SuspenseNode in resuspended trees. } else { - let primaryChild: Fiber | null = null; - const areSuspenseChildrenConditionallyWrapped = - OffscreenComponent === -1; - if (areSuspenseChildrenConditionallyWrapped) { - primaryChild = fiber.child; - } else if (fiber.child !== null) { - primaryChild = fiber.child.child; - updateTrackedPathStateBeforeMount(fiber.child, null); - } + const primaryChild: Fiber | null = fiber.child; if (primaryChild !== null) { mountChildrenRecursively( primaryChild, @@ -3209,6 +3223,21 @@ export function attach( virtualInstance.treeBaseDuration = treeBaseDuration; } + function addUnfilteredChildrenIDs( + parentInstance: DevToolsInstance, + nextChildren: Array, + ): void { + let child: null | DevToolsInstance = parentInstance.firstChild; + while (child !== null) { + if (child.kind === FILTERED_FIBER_INSTANCE) { + addUnfilteredChildrenIDs(child, nextChildren); + } else { + nextChildren.push(child.id); + } + child = child.nextSibling; + } + } + function recordResetChildren( parentInstance: FiberInstance | VirtualInstance, ) { @@ -3226,21 +3255,7 @@ export function attach( // This is trickier than a simple comparison though, since certain types of fibers are filtered. const nextChildren: Array = []; - let child: null | DevToolsInstance = parentInstance.firstChild; - while (child !== null) { - if (child.kind === FILTERED_FIBER_INSTANCE) { - for ( - let innerChild: null | DevToolsInstance = parentInstance.firstChild; - innerChild !== null; - innerChild = innerChild.nextSibling - ) { - nextChildren.push((innerChild: any).id); - } - } else { - nextChildren.push(child.id); - } - child = child.nextSibling; - } + addUnfilteredChildrenIDs(parentInstance, nextChildren); const numChildren = nextChildren.length; if (numChildren < 2) { @@ -3336,7 +3351,7 @@ export function attach( // so we assume insertSuspendedBy dedupes. insertSuspendedBy(asyncInfo); } - if (previousVirtualInstance) continue; + continue; } if (typeof debugEntry.name !== 'string') { // Not a Component. Some other Debug Info. @@ -3496,7 +3511,8 @@ export function attach( } if (existingInstance !== null) { // Common case. Match in the same parent. - const fiberInstance: FiberInstance = (existingInstance: any); // Only matches if it's a Fiber. + const fiberInstance: FiberInstance | FilteredFiberInstance = + (existingInstance: any); // Only matches if it's a Fiber. // We keep track if the order of the children matches the previous order. // They are always different referentially, but if the instances line up @@ -3604,7 +3620,7 @@ export function attach( // Returns whether closest unfiltered fiber parent needs to reset its child list. function updateFiberRecursively( - fiberInstance: null | FiberInstance, // null if this should be filtered + fiberInstance: null | FiberInstance | FilteredFiberInstance, // null if this should be filtered nextFiber: Fiber, prevFiber: Fiber, traceNearestHostComponentUpdate: boolean, @@ -3703,9 +3719,9 @@ export function attach( aquireHostInstance(nearestInstance, nextFiber.stateNode); } - const isSuspense = nextFiber.tag === SuspenseComponent; let shouldResetChildren = false; - // The behavior of timed-out Suspense trees is unique. + + // 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), // React re-parents this content within a hidden Fragment while the fallback is showing. // This behavior doesn't need to be observable in the DevTools though. @@ -3713,8 +3729,17 @@ export function attach( // The easiest fix is to strip out the intermediate Fragment fibers, // so the Elements panel and Profiler don't need to special case them. // Suspense components only have a non-null memoizedState if they're timed-out. - const prevDidTimeout = isSuspense && prevFiber.memoizedState !== null; - const nextDidTimeOut = isSuspense && nextFiber.memoizedState !== null; + const isLegacySuspense = + nextFiber.tag === SuspenseComponent && OffscreenComponent === -1; + const prevDidTimeout = + isLegacySuspense && prevFiber.memoizedState !== null; + const nextDidTimeOut = + isLegacySuspense && nextFiber.memoizedState !== null; + + const isOffscreen = nextFiber.tag === OffscreenComponent; + const prevWasHidden = isOffscreen && prevFiber.memoizedState !== null; + const nextIsHidden = isOffscreen && nextFiber.memoizedState !== null; + // The logic below is inspired by the code paths in updateSuspenseComponent() // inside ReactFiberBeginWork in the React source code. if (prevDidTimeout && nextDidTimeOut) { @@ -3781,6 +3806,25 @@ export function attach( ); shouldResetChildren = true; } + } else if (prevWasHidden && nextIsHidden) { + // We don't update any children while they're still hidden. + } else if (prevWasHidden && !nextIsHidden) { + // We're revealing the hidden children. We now need to update them to the latest state. + if (nextFiber.child !== null) { + mountChildrenRecursively( + nextFiber.child, + traceNearestHostComponentUpdate, + ); + shouldResetChildren = true; + } + } else if (!prevWasHidden && nextIsHidden) { + // We're hiding the children. We really just unmount them for now. + updateChildrenRecursively( + null, + prevFiber.child, + traceNearestHostComponentUpdate, + ); + shouldResetChildren = true; } else { // Common case: Primary -> Primary. // This is the same code path as for non-Suspense fibers. @@ -3830,26 +3874,31 @@ export function attach( if (fiberInstance !== null) { removePreviousSuspendedBy(fiberInstance, previousSuspendedBy); - let componentLogsEntry = fiberToComponentLogsMap.get( - fiberInstance.data, - ); - if (componentLogsEntry === undefined && fiberInstance.data.alternate) { - componentLogsEntry = fiberToComponentLogsMap.get( - fiberInstance.data.alternate, + if (fiberInstance.kind === FIBER_INSTANCE) { + let componentLogsEntry = fiberToComponentLogsMap.get( + fiberInstance.data, ); - } - recordConsoleLogs(fiberInstance, componentLogsEntry); + if ( + componentLogsEntry === undefined && + fiberInstance.data.alternate + ) { + componentLogsEntry = fiberToComponentLogsMap.get( + fiberInstance.data.alternate, + ); + } + recordConsoleLogs(fiberInstance, componentLogsEntry); - const isProfilingSupported = - nextFiber.hasOwnProperty('treeBaseDuration'); - if (isProfilingSupported) { - recordProfilingDurations(fiberInstance, prevFiber); + const isProfilingSupported = + nextFiber.hasOwnProperty('treeBaseDuration'); + if (isProfilingSupported) { + recordProfilingDurations(fiberInstance, prevFiber); + } } } if (shouldResetChildren) { // 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) { + 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. From c260b38d0a082641342fc45ff5ac96e32f764f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 31 Jul 2025 10:30:31 -0400 Subject: [PATCH 2/2] [DevTools] Clean up Virtual Instances from id map (#34063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was a pretty glaring memory leak. 🙈 I forgot to clean up the VirtualInstances from the id map so the Server Component instances always leaked in DEV. --- packages/react-devtools-shared/src/backend/fiber/renderer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 11346e7ec31..5abfcc8c8b7 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -2749,6 +2749,8 @@ export function attach( const id = instance.id; pendingRealUnmountedIDs.push(id); + + idToDevToolsInstanceMap.delete(instance.id); } function getSecondaryEnvironmentName(