From 01c8a88d57d23653d4352f2a2556a1274dc48a81 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 15 May 2026 12:52:40 -0700 Subject: [PATCH] [DevTools] Fix double-REMOVE crash when Activity hidden children are inside a suspended Subtree (#36315, #36375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error "Cannot remove node X because no matching node was found in the Store" was introduced by the Activity hidden-subtree feature (ed69815ce), which made Activity's hidden children visible in the DevTools Store. ## Root cause When a Suspense boundary suspends, `disconnectChildrenRecursively` walks the DevTools instance tree and sends TREE_OPERATION_REMOVE for every visible node — including children of hidden Activity components, which are now tracked in the Store. The backend FiberInstances remain alive (React keeps the fiber tree for the suspended content), so their IDs are still in `idToDevToolsInstanceMap`. Later, if one of those children is removed from the React tree while the Suspense is still suspended, `unmountRemainingChildren()` is called from inside `updateFiberRecursively`'s `finally` block. At that point the `isInDisconnectedSubtree` module-level flag has already been restored to `false` by an inner try/finally, and the Activity's Offscreen fiber does not satisfy `isSuspendedOffscreen()` (its parent is ActivityComponent, not SuspenseComponent), so `unmountRemainingChildren` takes the unguarded else branch and calls `recordDisconnect` again for the same instance — sending a second TREE_OPERATION_REMOVE for a node that is no longer in `_idToElement`. ## Fix Add a per-instance `isDisconnected` boolean to `FiberInstance` and `VirtualInstance` (initialized to `false`). This acts as a second, stateful guard independent of the global `isInDisconnectedSubtree` flag: - `recordDisconnect` / `recordVirtualDisconnect`: return early if `instance.isDisconnected` is already `true`; set it to `true` before pushing to `pendingRealUnmountedIDs`. - `recordReconnect` / `recordVirtualReconnect`: reset `isDisconnected` to `false` so that the normal suspend → reveal → suspend cycle continues to work correctly. The per-instance flag prevents any double-REMOVE regardless of which code path causes `recordDisconnect` to be called a second time, making the fix robust against future interactions with complex Suspense / Activity trees. Co-Authored-By: Claude Sonnet 4.6 --- .../src/__tests__/store-test.js | 72 +++++++++++++++++++ .../src/backend/fiber/renderer.js | 18 +++++ .../fiber/shared/DevToolsFiberTypes.js | 2 + 3 files changed, 92 insertions(+) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 0a62c089dd13..a848d04750fc 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -558,6 +558,78 @@ describe('Store', () => { expect(leaf.displayName).toBe('Leaf'); expect(leaf.isInsideHiddenActivity).toBe(true); }); + + // Regression test for https://github.com/facebook/react/issues/36315 + // and https://github.com/facebook/react/issues/36375. + // @reactVersion >= 19 + it('should not throw when a child of hidden Activity is removed while a parent Suspense is suspended', async () => { + const Activity = React.Activity || React.unstable_Activity; + + let resolve; + const never = new Promise(_resolve => { + resolve = _resolve; + }); + + function Suspender() { + readValue(never); + return null; + } + + function Child() { + return
child
; + } + + function App({showChild, suspend}) { + return ( + Loading}> + {suspend ? : null} + {showChild ? : null} + + ); + } + + // Mount with child visible inside hidden Activity (child is in the Store) + await actAsync(() => { + render(); + }); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + ▸ + [suspense-root] rects={[{x:1,y:2,width:5,height:1}]} + + `); + + // Suspend the Suspense — this sends REMOVE for Activity and Child + await actAsync(() => { + render(); + }); + + // Remove Child from Activity's content while Suspense is still suspended. + // This must NOT send a second REMOVE for Child (which is no longer in the Store). + await actAsync(() => { + render(); + }); + + // Reveal Suspense — Activity and any remaining children should be re-added + await actAsync(() => { + render(); + }); + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + [suspense-root] rects={null} + + `); + + // Resolve the promise to clean up + await actAsync(() => { + resolve(); + }); + }); }); describe('collapseNodesByDefault:false', () => { diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 037ce1c5cc3b..1f4e1773dc8a 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -209,6 +209,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance { treeBaseDuration: 0, suspendedBy: null, suspenseNode: null, + isDisconnected: false, data: fiber, }; } @@ -244,6 +245,7 @@ function createVirtualInstance( treeBaseDuration: 0, suspendedBy: null, suspenseNode: null, + isDisconnected: false, data: debugEntry, }; } @@ -1784,6 +1786,7 @@ export function attach( // We're disconnected. We'll reconnect a hidden mount after the parent reappears. return; } + fiberInstance.isDisconnected = false; const id = fiberInstance.id; const fiber = fiberInstance.data; @@ -1968,6 +1971,7 @@ export function attach( // We're disconnected. We'll reconnect a hidden mount after the parent reappears. return; } + instance.isDisconnected = false; const componentInfo = instance.data; const key = @@ -2128,6 +2132,13 @@ export function attach( return; } + if (fiberInstance.isDisconnected) { + // A REMOVE was already sent for this instance (e.g. via disconnectChildrenRecursively + // when a parent Suspense suspended). Don't send a second REMOVE. + return; + } + fiberInstance.isDisconnected = true; + if (trackedPathMatchInstance === fiberInstance) { // We're in the process of trying to restore previous selection. // If this fiber matched but is being hidden, there's no use trying. @@ -2853,6 +2864,13 @@ export function attach( if (isInDisconnectedSubtree) { return; } + + if (instance.isDisconnected) { + // A REMOVE was already sent for this instance. Don't send a second REMOVE. + return; + } + instance.isDisconnected = true; + if (trackedPathMatchInstance === instance) { // We're in the process of trying to restore previous selection. // If this fiber matched but is being unmounted, there's no use trying. diff --git a/packages/react-devtools-shared/src/backend/fiber/shared/DevToolsFiberTypes.js b/packages/react-devtools-shared/src/backend/fiber/shared/DevToolsFiberTypes.js index 1b01dfbc5bbc..046f1fd67bd6 100644 --- a/packages/react-devtools-shared/src/backend/fiber/shared/DevToolsFiberTypes.js +++ b/packages/react-devtools-shared/src/backend/fiber/shared/DevToolsFiberTypes.js @@ -34,6 +34,7 @@ export type FiberInstance = { treeBaseDuration: number, // the profiled time of the last render of this subtree suspendedBy: null | Array, // things that suspended in the children position of this component suspenseNode: null | SuspenseNode, + isDisconnected: boolean, // true if a REMOVE was sent to the frontend for this instance data: Fiber, // one of a Fiber pair }; @@ -69,6 +70,7 @@ export type VirtualInstance = { treeBaseDuration: number, // the profiled time of the last render of this subtree suspendedBy: null | Array, // things that blocked the server component's child from rendering suspenseNode: null, + isDisconnected: boolean, // true if a REMOVE was sent to the frontend for this instance // The latest info for this instance. This can be updated over time and the // same info can appear in more than once ServerComponentInstance. data: ReactComponentInfo,