From d80b761b8f9f68f15a0fcbe6443e62812540f81c Mon Sep 17 00:00:00 2001
From: Luna Ruan <lunaris.ruan@gmail.com>
Date: Thu, 20 Jan 2022 16:29:43 -0500
Subject: [PATCH] Filter out deleted components that are added to the updaters
 list (#23156)

There was a bug that occurred when a destroy effect is called that causes an update. The update would be added to the updaters list even though the fiber that was calling the destroy effect was unmounted and no longer exists. This PR:

* Adds a patch to Devtools to filter out all in the update list that aren't in the fiberToIDMap (which contains all fibers currently on screen)
---
 .../src/__tests__/profilingCache-test.js      | 93 +++++++++++++++++++
 .../src/backend/renderer.js                   |  4 +-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js
index 00605d857a153..67db757461a6d 100644
--- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js
+++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js
@@ -752,4 +752,97 @@ describe('ProfilingCache', () => {
     utils.act(() => store.profilerStore.stopProfiling());
     expect(container.textContent).toBe('About');
   });
+
+  it('components that were deleted and added to updaters during the layout phase should not crash', () => {
+    let setChildUnmounted;
+    function Child() {
+      const [, setState] = React.useState(false);
+
+      React.useLayoutEffect(() => {
+        return () => setState(true);
+      });
+
+      return null;
+    }
+
+    function App() {
+      const [childUnmounted, _setChildUnmounted] = React.useState(false);
+      setChildUnmounted = _setChildUnmounted;
+      return <>{!childUnmounted && <Child />}</>;
+    }
+
+    const root = ReactDOM.createRoot(document.createElement('div'));
+    utils.act(() => root.render(<App />));
+    utils.act(() => store.profilerStore.startProfiling());
+    utils.act(() => setChildUnmounted(true));
+    utils.act(() => store.profilerStore.stopProfiling());
+
+    const updaters = store.profilerStore.getCommitData(store.roots[0], 0)
+      .updaters;
+    expect(updaters.length).toEqual(1);
+    expect(updaters[0].displayName).toEqual('App');
+  });
+
+  it('components in a deleted subtree and added to updaters during the layout phase should not crash', () => {
+    let setChildUnmounted;
+    function Child() {
+      return <GrandChild />;
+    }
+
+    function GrandChild() {
+      const [, setState] = React.useState(false);
+
+      React.useLayoutEffect(() => {
+        return () => setState(true);
+      });
+
+      return null;
+    }
+
+    function App() {
+      const [childUnmounted, _setChildUnmounted] = React.useState(false);
+      setChildUnmounted = _setChildUnmounted;
+      return <>{!childUnmounted && <Child />}</>;
+    }
+
+    const root = ReactDOM.createRoot(document.createElement('div'));
+    utils.act(() => root.render(<App />));
+    utils.act(() => store.profilerStore.startProfiling());
+    utils.act(() => setChildUnmounted(true));
+    utils.act(() => store.profilerStore.stopProfiling());
+
+    const updaters = store.profilerStore.getCommitData(store.roots[0], 0)
+      .updaters;
+    expect(updaters.length).toEqual(1);
+    expect(updaters[0].displayName).toEqual('App');
+  });
+
+  it('components that were deleted should not be added to updaters during the passive phase', () => {
+    let setChildUnmounted;
+    function Child() {
+      const [, setState] = React.useState(false);
+      React.useEffect(() => {
+        return () => setState(true);
+      });
+
+      return null;
+    }
+
+    function App() {
+      const [childUnmounted, _setChildUnmounted] = React.useState(false);
+      setChildUnmounted = _setChildUnmounted;
+      return <>{!childUnmounted && <Child />}</>;
+    }
+
+    const root = ReactDOM.createRoot(document.createElement('div'));
+    utils.act(() => root.render(<App />));
+    utils.act(() => store.profilerStore.startProfiling());
+    utils.act(() => setChildUnmounted(true));
+    utils.act(() => store.profilerStore.stopProfiling());
+
+    const updaters = store.profilerStore.getCommitData(store.roots[0], 0)
+      .updaters;
+    expect(updaters.length).toEqual(1);
+    expect(updaters[0].displayName).toEqual('App');
+  });
 });
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
index daa4d4e1030c3..0f7b15cd019b9 100644
--- a/packages/react-devtools-shared/src/backend/renderer.js
+++ b/packages/react-devtools-shared/src/backend/renderer.js
@@ -2586,7 +2586,9 @@ export function attach(
 
   function getUpdatersList(root): Array<SerializedElement> | null {
     return root.memoizedUpdaters != null
-      ? Array.from(root.memoizedUpdaters).map(fiberToSerializedElement)
+      ? Array.from(root.memoizedUpdaters)
+          .filter(fiber => getFiberIDUnsafe(fiber) !== null)
+          .map(fiberToSerializedElement)
       : null;
   }