From 9b939d76a3eab397a8c71cb2dee9f5b4a2112087 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Fri, 12 Sep 2025 11:50:44 +0100 Subject: [PATCH 1/4] tests: add a failing test for suspense store reconciliation --- .../src/__tests__/store-test.js | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 3b60d5ae093e4..e0b2a43ed52bc 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -3081,6 +3081,109 @@ describe('Store', () => { `); }); + // @reactVersion >= 19.0 + it('can reconcile Suspense inside an Offscreen', async () => { + let resolveHeadFallback; + let resolveHeadContent; + let resolveMainFallback; + let resolveMainContent; + + function Component({children, promise}) { + if (promise) { + React.use(promise); + } + return
{children}
; + } + + function WithSuspenseInFallback({fallbackPromise, contentPromise, name}) { + return ( + + Loading fallback... + + }> + + Loading... + + + }> + + done + + + ); + } + + function App({ + headFallbackPromise, + headContentPromise, + mainContentPromise, + mainFallbackPromise, + tailContentPromise, + tailFallbackPromise, + }) { + return ( + + + + + ); + } + + const initialHeadContentPromise = new Promise(resolve => { + resolveHeadContent = resolve; + }); + const initialHeadFallbackPromise = new Promise(resolve => { + resolveHeadFallback = resolve; + }); + const initialMainContentPromise = new Promise(resolve => { + resolveMainContent = resolve; + }); + const initialMainFallbackPromise = new Promise(resolve => { + resolveMainFallback = resolve; + }); + await actAsync(() => + render( + , + ), + ); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + [suspense-root] rects={[{x:1,y:2,width:19,height:1}, {x:1,y:2,width:19,height:1}]} + + + + + `); + + await actAsync(() => render(null)); + + expect(store).toMatchInlineSnapshot(``); + }); + it('should handle an empty root', async () => { await actAsync(() => render(null)); expect(store).toMatchInlineSnapshot(``); From c4f7e099164ed57ba9ff099f6813c380efc1379f Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Fri, 12 Sep 2025 17:26:27 +0200 Subject: [PATCH 2/4] Simplify test --- .../src/__tests__/store-test.js | 101 +----------------- 1 file changed, 1 insertion(+), 100 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index e0b2a43ed52bc..218b088184ffa 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -3079,109 +3079,10 @@ describe('Store', () => { `); - }); - - // @reactVersion >= 19.0 - it('can reconcile Suspense inside an Offscreen', async () => { - let resolveHeadFallback; - let resolveHeadContent; - let resolveMainFallback; - let resolveMainContent; - - function Component({children, promise}) { - if (promise) { - React.use(promise); - } - return
{children}
; - } - - function WithSuspenseInFallback({fallbackPromise, contentPromise, name}) { - return ( - - Loading fallback... - - }> - - Loading... - - - }> - - done - - - ); - } - - function App({ - headFallbackPromise, - headContentPromise, - mainContentPromise, - mainFallbackPromise, - tailContentPromise, - tailFallbackPromise, - }) { - return ( - - - - - ); - } - - const initialHeadContentPromise = new Promise(resolve => { - resolveHeadContent = resolve; - }); - const initialHeadFallbackPromise = new Promise(resolve => { - resolveHeadFallback = resolve; - }); - const initialMainContentPromise = new Promise(resolve => { - resolveMainContent = resolve; - }); - const initialMainFallbackPromise = new Promise(resolve => { - resolveMainFallback = resolve; - }); - await actAsync(() => - render( - , - ), - ); - - expect(store).toMatchInlineSnapshot(` - [root] - ▾ - - [suspense-root] rects={[{x:1,y:2,width:19,height:1}, {x:1,y:2,width:19,height:1}]} - - - - - `); await actAsync(() => render(null)); - expect(store).toMatchInlineSnapshot(``); + expect(store).toMatchInlineSnapshot(); }); it('should handle an empty root', async () => { From 500a0563d6db74d6a215f90fa607c02348e3b1d0 Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Fri, 12 Sep 2025 17:55:36 +0200 Subject: [PATCH 3/4] [DevTools] Unmount fallbacks in the context of the parent Suspense --- .../src/__tests__/store-test.js | 2 +- .../src/backend/fiber/renderer.js | 67 ++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 218b088184ffa..88144b4aa1c13 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -3082,7 +3082,7 @@ describe('Store', () => { await actAsync(() => render(null)); - expect(store).toMatchInlineSnapshot(); + expect(store).toMatchInlineSnapshot(``); }); it('should handle an empty root', async () => { diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index aee89e8ca2c54..a63d7ee12f5e8 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -3070,6 +3070,33 @@ export function attach( } } + function unmountSuspenseChildrenRecursively( + contentInstance: DevToolsInstance, + stashedSuspenseParent: null | SuspenseNode, + stashedSuspensePrevious: null | SuspenseNode, + stashedSuspenseRemaining: null | SuspenseNode, + ): void { + // First unmount only the Offscreen boundary. I.e. the main content. + unmountInstanceRecursively(contentInstance); + + // Next, we'll pop back out of the SuspenseNode that we added above and now we'll + // unmount the fallback, unmounting anything in the context of the parent SuspenseNode. + // Since the fallback conceptually blocks the parent. + reconcilingParentSuspenseNode = stashedSuspenseParent; + previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; + remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; + const fallbackInstance = remainingReconcilingChildren; + if (fallbackInstance !== null) { + unmountInstanceRecursively(fallbackInstance); + + if (remainingReconcilingChildren !== null) { + throw new Error( + 'There should be no instances left to unmount after a Suspense fallback was unmounted.', + ); + } + } + } + function isChildOf( parentInstance: DevToolsInstance, childInstance: DevToolsInstance, @@ -4015,6 +4042,7 @@ export function attach( debug('unmountInstanceRecursively()', instance, reconcilingParent); } + let shouldPopSuspenseNode = false; const stashedParent = reconcilingParent; const stashedPrevious = previouslyReconciledSibling; const stashedRemaining = remainingReconcilingChildren; @@ -4035,11 +4063,46 @@ export function attach( previouslyReconciledSiblingSuspenseNode = null; remainingReconcilingChildrenSuspenseNodes = instance.suspenseNode.firstChild; + + shouldPopSuspenseNode = true; } try { // Unmount the remaining set. - unmountRemainingChildren(); + if ( + (instance.kind === FIBER_INSTANCE || + instance.kind === FILTERED_FIBER_INSTANCE) && + instance.data.tag === SuspenseComponent && + OffscreenComponent !== -1 + ) { + const fiber = instance.data; + const contentFiberInstance = remainingReconcilingChildren; + const hydrated = isFiberHydrated(fiber); + if (hydrated) { + if (contentFiberInstance === null) { + throw new Error( + 'There should always be an Offscreen Fiber child in a hydrated Suspense boundary.', + ); + } + + unmountSuspenseChildrenRecursively( + contentFiberInstance, + stashedSuspenseParent, + stashedSuspensePrevious, + stashedSuspenseRemaining, + ); + // unmountSuspenseChildren already popped + shouldPopSuspenseNode = false; + } else { + if (contentFiberInstance !== null) { + throw new Error( + 'A dehydrated Suspense node should not have a content Fiber.', + ); + } + } + } else { + unmountRemainingChildren(); + } removePreviousSuspendedBy( instance, previousSuspendedBy, @@ -4049,7 +4112,7 @@ export function attach( reconcilingParent = stashedParent; previouslyReconciledSibling = stashedPrevious; remainingReconcilingChildren = stashedRemaining; - if (instance.suspenseNode !== null) { + if (shouldPopSuspenseNode) { reconcilingParentSuspenseNode = stashedSuspenseParent; previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; From 253fb903d820797cc7ddd5e328f0e63b515bdc7e Mon Sep 17 00:00:00 2001 From: Sebastian Sebbie Silbermann Date: Fri, 12 Sep 2025 18:15:38 +0200 Subject: [PATCH 4/4] There is no single fallback Fiber e.g. if you have a Fragment --- .../src/backend/fiber/renderer.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index a63d7ee12f5e8..eb86ffea713fa 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -3085,16 +3085,7 @@ export function attach( reconcilingParentSuspenseNode = stashedSuspenseParent; previouslyReconciledSiblingSuspenseNode = stashedSuspensePrevious; remainingReconcilingChildrenSuspenseNodes = stashedSuspenseRemaining; - const fallbackInstance = remainingReconcilingChildren; - if (fallbackInstance !== null) { - unmountInstanceRecursively(fallbackInstance); - - if (remainingReconcilingChildren !== null) { - throw new Error( - 'There should be no instances left to unmount after a Suspense fallback was unmounted.', - ); - } - } + unmountRemainingChildren(); } function isChildOf(