Implement "best renderer" by taking the inner most matched node

Don't leak the "Fiber" type outside the renderer interface.
facebook · Jul 30, 2024 · f800bef · f800bef
1 parent 33e54fa
commit f800bef
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 46 deletions.
diff --git a/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js b/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js
@@ -905,7 +905,7 @@ describe('Store (legacy)', () => {
         [root]
           ▸ <Wrapper>
       `);
-        const deepestedNodeID = global.agent.getIDForNode(ref);
+        const deepestedNodeID = global.agent.getIDForHostInstance(ref);
         act(() => store.toggleIsCollapsed(deepestedNodeID, false));
         expect(store).toMatchInlineSnapshot(`
         [root]

diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js
@@ -1217,7 +1217,7 @@ describe('Store', () => {
           ▸ <Wrapper>
       `);
 
-      const deepestedNodeID = agent.getIDForNode(ref.current);
+      const deepestedNodeID = agent.getIDForHostInstance(ref.current);
 
       act(() => store.toggleIsCollapsed(deepestedNodeID, false));
       expect(store).toMatchInlineSnapshot(`

diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
@@ -43,7 +43,7 @@ import type {
   ComponentFilter,
   BrowserTheme,
 } from 'react-devtools-shared/src/frontend/types';
-import {isSynchronousXHRSupported} from './utils';
+import {isSynchronousXHRSupported, isReactNativeEnvironment} from './utils';
 
 const debug = (methodName: string, ...args: Array<string>) => {
   if (__DEBUG__) {
@@ -343,31 +343,79 @@ export default class Agent extends EventEmitter<{
     return renderer.getInstanceAndStyle(id);
   }
 
-  getBestMatchingRendererInterface(node: Object): RendererInterface | null {
-    let bestMatch = null;
+  getIDForHostInstance(target: HostInstance): number | null {
+    let bestMatch: null | HostInstance = null;
+    let bestRenderer: null | RendererInterface = null;
+    // Find the nearest ancestor which is mounted by a React.
     for (const rendererID in this._rendererInterfaces) {
       const renderer = ((this._rendererInterfaces[
         (rendererID: any)
       ]: any): RendererInterface);
-      const fiber = renderer.getFiberForNative(node);
-      if (fiber !== null) {
-        // check if fiber.stateNode is matching the original hostInstance
-        if (fiber.stateNode === node) {
-          return renderer;
-        } else if (bestMatch === null) {
-          bestMatch = renderer;
+      const nearestNode: null = renderer.getNearestMountedHostInstance(target);
+      if (nearestNode !== null) {
+        if (nearestNode === target) {
+          // Exact match we can exit early.
+          bestMatch = nearestNode;
+          bestRenderer = renderer;
+          break;
+        }
+        if (
+          bestMatch === null ||
+          (!isReactNativeEnvironment() && bestMatch.contains(nearestNode))
+        ) {
+          // If this is the first match or the previous match contains the new match,
+          // so the new match is a deeper and therefore better match.
+          bestMatch = nearestNode;
+          bestRenderer = renderer;
         }
       }
     }
-    // if an exact match is not found, return the first valid renderer as fallback
-    return bestMatch;
+    if (bestRenderer != null && bestMatch != null) {
+      try {
+        return bestRenderer.getElementIDForHostInstance(bestMatch, true);
+      } catch (error) {
+        // Some old React versions might throw if they can't find a match.
+        // If so we should ignore it...
+      }
+    }
+    return null;
   }
 
-  getIDForNode(node: Object): number | null {
-    const rendererInterface = this.getBestMatchingRendererInterface(node);
-    if (rendererInterface != null) {
+  getComponentNameForHostInstance(target: HostInstance): string | null {
+    // We duplicate this code from getIDForHostInstance to avoid an object allocation.
+    let bestMatch: null | HostInstance = null;
+    let bestRenderer: null | RendererInterface = null;
+    // Find the nearest ancestor which is mounted by a React.
+    for (const rendererID in this._rendererInterfaces) {
+      const renderer = ((this._rendererInterfaces[
+        (rendererID: any)
+      ]: any): RendererInterface);
+      const nearestNode = renderer.getNearestMountedHostInstance(target);
+      if (nearestNode !== null) {
+        if (nearestNode === target) {
+          // Exact match we can exit early.
+          bestMatch = nearestNode;
+          bestRenderer = renderer;
+          break;
+        }
+        if (
+          bestMatch === null ||
+          (!isReactNativeEnvironment() && bestMatch.contains(nearestNode))
+        ) {
+          // If this is the first match or the previous match contains the new match,
+          // so the new match is a deeper and therefore better match.
+          bestMatch = nearestNode;
+          bestRenderer = renderer;
+        }
+      }
+    }
+
+    if (bestRenderer != null && bestMatch != null) {
       try {
-        return rendererInterface.getElementIDForHostInstance(node, true);
+        const id = bestRenderer.getElementIDForHostInstance(bestMatch, true);
+        if (id) {
+          return bestRenderer.getDisplayNameForElementID(id);
+        }
       } catch (error) {
         // Some old React versions might throw if they can't find a match.
         // If so we should ignore it...
@@ -616,8 +664,8 @@ export default class Agent extends EventEmitter<{
     }
   };
 
-  selectNode(target: Object): void {
-    const id = this.getIDForNode(target);
+  selectNode(target: HostInstance): void {
+    const id = this.getIDForHostInstance(target);
     if (id !== null) {
       this._bridge.send('selectElement', id);
     }

diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -2882,8 +2882,14 @@ export function attach(
     return fiber != null ? getDisplayNameForFiber(fiber) : null;
   }
 
-  function getFiberForNative(hostInstance: HostInstance) {
-    return renderer.findFiberByHostInstance(hostInstance);
+  function getNearestMountedHostInstance(
+    hostInstance: HostInstance,
+  ): null | HostInstance {
+    const mountedHostInstance = renderer.findFiberByHostInstance(hostInstance);
+    if (mountedHostInstance != null) {
+      return mountedHostInstance.stateNode;
+    }
+    return null;
   }
 
   function getElementIDForHostInstance(
@@ -4659,7 +4665,7 @@ export function attach(
     flushInitialOperations,
     getBestMatchForTrackedPath,
     getDisplayNameForElementID,
-    getFiberForNative,
+    getNearestMountedHostInstance,
     getElementIDForHostInstance,
     getInstanceAndStyle,
     getOwnersList,

diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js
@@ -145,7 +145,9 @@ export function attach(
   let getElementIDForHostInstance: GetElementIDForHostInstance =
     ((null: any): GetElementIDForHostInstance);
   let findHostInstanceForInternalID: (id: number) => ?HostInstance;
-  let getFiberForNative = (node: HostInstance) => {
+  let getNearestMountedHostInstance = (
+    node: HostInstance,
+  ): null | HostInstance => {
     // Not implemented.
     return null;
   };
@@ -160,8 +162,15 @@ export function attach(
       const internalInstance = idToInternalInstanceMap.get(id);
       return renderer.ComponentTree.getNodeFromInstance(internalInstance);
     };
-    getFiberForNative = (node: HostInstance) => {
-      return renderer.ComponentTree.getClosestInstanceFromNode(node);
+    getNearestMountedHostInstance = (
+      node: HostInstance,
+    ): null | HostInstance => {
+      const internalInstance =
+        renderer.ComponentTree.getClosestInstanceFromNode(node);
+      if (internalInstance != null) {
+        return renderer.ComponentTree.getNodeFromInstance(internalInstance);
+      }
+      return null;
     };
   } else if (renderer.Mount.getID && renderer.Mount.getNode) {
     getElementIDForHostInstance = (node, findNearestUnfilteredAncestor) => {
@@ -1111,7 +1120,7 @@ export function attach(
     flushInitialOperations,
     getBestMatchForTrackedPath,
     getDisplayNameForElementID,
-    getFiberForNative,
+    getNearestMountedHostInstance,
     getElementIDForHostInstance,
     getInstanceAndStyle,
     findHostInstancesForElementID: (id: number) => {

diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -86,10 +86,7 @@ type SharedInternalsSubset = {
 };
 export type CurrentDispatcherRef = SharedInternalsSubset;
 
-export type GetDisplayNameForElementID = (
-  id: number,
-  findNearestUnfilteredAncestor?: boolean,
-) => string | null;
+export type GetDisplayNameForElementID = (id: number) => string | null;
 
 export type GetElementIDForHostInstance = (
   component: HostInstance,
@@ -363,7 +360,9 @@ export type RendererInterface = {
   findHostInstancesForElementID: FindHostInstancesForElementID,
   flushInitialOperations: () => void,
   getBestMatchForTrackedPath: () => PathMatch | null,
-  getFiberForNative: (component: HostInstance) => Fiber | null,
+  getNearestMountedHostInstance: (
+    component: HostInstance,
+  ) => HostInstance | null,
   getElementIDForHostInstance: GetElementIDForHostInstance,
   getDisplayNameForElementID: GetDisplayNameForElementID,
   getInstanceAndStyle(id: number): InstanceAndStyle,

diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js b/packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js
@@ -233,19 +233,9 @@ export default class Overlay {
       name = elements[0].nodeName.toLowerCase();
 
       const node = elements[0];
-      const rendererInterface =
-        this.agent.getBestMatchingRendererInterface(node);
-      if (rendererInterface) {
-        const id = rendererInterface.getElementIDForHostInstance(node, true);
-        if (id) {
-          const ownerName = rendererInterface.getDisplayNameForElementID(
-            id,
-            true,
-          );
-          if (ownerName) {
-            name += ' (in ' + ownerName + ')';
-          }
-        }
+      const ownerName = this.agent.getComponentNameForHostInstance(node);
+      if (ownerName) {
+        name += ' (in ' + ownerName + ')';
       }
     }
 

diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js
@@ -193,7 +193,7 @@ export default function setupHighlighter(
 
   const selectElementForNode = throttle(
     memoize((node: HTMLElement) => {
-      const id = agent.getIDForNode(node);
+      const id = agent.getIDForHostInstance(node);
       if (id !== null) {
         bridge.send('selectElement', id);
       }