From 77e2ba166812d1ef522021dcdc4b1a1b94f68a11 Mon Sep 17 00:00:00 2001 From: Luna Date: Wed, 6 Jul 2022 18:31:22 -0400 Subject: [PATCH] fix devtools perf issue and added regression test --- .../src/__tests__/store-test.js | 44 +++++++++++++++++++ .../src/backend/renderer.js | 17 +++---- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 56795d232895c..a6dc02f844e5f 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -13,6 +13,7 @@ describe('Store', () => { let ReactDOMClient; let agent; let act; + let actAsync; let bridge; let getRendererID; let legacyRender; @@ -30,6 +31,7 @@ describe('Store', () => { const utils = require('./utils'); act = utils.act; + actAsync = utils.actAsync; getRendererID = utils.getRendererID; legacyRender = utils.legacyRender; withErrorsOrWarningsIgnored = utils.withErrorsOrWarningsIgnored; @@ -2064,5 +2066,47 @@ describe('Store', () => { expect(store.errorCount).toBe(0); expect(store.warningCount).toBe(0); }); + + // Regression test for https://github.com/facebook/react/issues/23202 + // @reactVersion >= 18.0 + it('suspense boundary children should not double unmount and error', async () => { + async function fakeImport(result) { + return {default: result}; + } + + const ChildA = () => null; + const ChildB = () => null; + + const LazyChildA = React.lazy(() => fakeImport(ChildA)); + const LazyChildB = React.lazy(() => fakeImport(ChildB)); + + function App({renderA}) { + return ( + + {renderA ? : } + + ); + } + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await actAsync(() => root.render()); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + `); + + await actAsync(() => root.render()); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + `); + }); }); }); diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 9959cad7aa525..f2d6dad45f002 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -2632,14 +2632,15 @@ export function attach( } function handleCommitFiberUnmount(fiber) { - // Flush any pending Fibers that we are untracking before processing the new commit. - // If we don't do this, we might end up double-deleting Fibers in some cases (like Legacy Suspense). - untrackFibers(); - - // This is not recursive. - // We can't traverse fibers after unmounting so instead - // we rely on React telling us about each unmount. - recordUnmount(fiber, false); + // If the untrackFiberSet already has the unmounted Fiber, this means we've already + // recordedUnmount, so we don't need to do it again. If we don't do this, we might + // end up double-deleting Fibers in some cases (like Legacy Suspense). + if (!untrackFibersSet.has(fiber)) { + // This is not recursive. + // We can't traverse fibers after unmounting so instead + // we rely on React telling us about each unmount. + recordUnmount(fiber, false); + } } function handlePostCommitFiberRoot(root) {