From 645ec5d6fce7926e1d23105a93621147ec3d7f17 Mon Sep 17 00:00:00 2001
From: Luna Ruan <lunaris.ruan@gmail.com>
Date: Thu, 17 Mar 2022 15:40:03 -0400
Subject: [PATCH] fix inspecting an element in a nested renderer bug (#24116)

Fixes this issue, where inspecting components in nested renderers results in an error. The reason for this is because we have different fiberToIDMap instances for each renderer, and owners of a component could be in different renderers.

This fix moves the fiberToIDMap and idToArbitraryFiberMap out of the attach method so there's only one instance of each for all renderers.
---
 .../src/__tests__/inspectedElement-test.js    | 56 +++++++++++++++++++
 .../src/backend/renderer.js                   | 22 ++++----
 2 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
index e6f8c3277d3a2..919fbb059f76b 100644
--- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
+++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
@@ -2734,6 +2734,62 @@ describe('InspectedElement', () => {
     });
   });
 
+  it('inspecting nested renderers should not throw', async () => {
+    // Ignoring react art warnings
+    spyOn(console, 'error');
+    const ReactArt = require('react-art');
+    const ArtSVGMode = require('art/modes/svg');
+    const ARTCurrentMode = require('art/modes/current');
+    store.componentFilters = [];
+
+    ARTCurrentMode.setCurrent(ArtSVGMode);
+    const {Surface, Group} = ReactArt;
+
+    function Child() {
+      return (
+        <Surface width={1} height={1}>
+          <Group />
+        </Surface>
+      );
+    }
+    function App() {
+      return <Child />;
+    }
+
+    await utils.actAsync(() => {
+      legacyRender(<App />, document.createElement('div'));
+    });
+    expect(store).toMatchInlineSnapshot(`
+      [root]
+        ▾ <App>
+          ▾ <Child>
+            ▾ <Surface>
+                <svg>
+      [root]
+          <Group>
+    `);
+
+    const inspectedElement = await inspectElementAtIndex(4);
+    expect(inspectedElement.owners).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "displayName": "Child",
+          "hocDisplayNames": null,
+          "id": 3,
+          "key": null,
+          "type": 5,
+        },
+        Object {
+          "displayName": "App",
+          "hocDisplayNames": null,
+          "id": 2,
+          "key": null,
+          "type": 5,
+        },
+      ]
+    `);
+  });
+
   describe('error boundary', () => {
     it('can toggle error', async () => {
       class LocalErrorBoundary extends React.Component<any> {
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
index 32ce8e2a9fe55..4b67a7a8ebe9c 100644
--- a/packages/react-devtools-shared/src/backend/renderer.js
+++ b/packages/react-devtools-shared/src/backend/renderer.js
@@ -548,6 +548,17 @@ export function getInternalReactConstants(
   };
 }
 
+// Map of one or more Fibers in a pair to their unique id number.
+// We track both Fibers to support Fast Refresh,
+// which may forcefully replace one of the pair as part of hot reloading.
+// In that case it's still important to be able to locate the previous ID during subsequent renders.
+const fiberToIDMap: Map<Fiber, number> = new Map();
+
+// Map of id to one (arbitrary) Fiber in a pair.
+// This Map is used to e.g. get the display name for a Fiber or schedule an update,
+// operations that should be the same whether the current and work-in-progress Fiber is used.
+const idToArbitraryFiberMap: Map<number, Fiber> = new Map();
+
 export function attach(
   hook: DevToolsHook,
   rendererID: number,
@@ -1085,17 +1096,6 @@ export function attach(
     }
   }
 
-  // Map of one or more Fibers in a pair to their unique id number.
-  // We track both Fibers to support Fast Refresh,
-  // which may forcefully replace one of the pair as part of hot reloading.
-  // In that case it's still important to be able to locate the previous ID during subsequent renders.
-  const fiberToIDMap: Map<Fiber, number> = new Map();
-
-  // Map of id to one (arbitrary) Fiber in a pair.
-  // This Map is used to e.g. get the display name for a Fiber or schedule an update,
-  // operations that should be the same whether the current and work-in-progress Fiber is used.
-  const idToArbitraryFiberMap: Map<number, Fiber> = new Map();
-
   // When profiling is supported, we store the latest tree base durations for each Fiber.
   // This is so that we can quickly capture a snapshot of those values if profiling starts.
   // If we didn't store these values, we'd have to crawl the tree when profiling started,