[DevTools] Implement "best renderer" by taking the inner most matched…

… node (#30494)

Stacked on #30491.

When going from DOM Node to select a component or highlight a component
we find the nearest mounted ancestor. However, when multiple renderers
are nested there can be multiple ancestors. The original fix #24665 did
this by taking the inner renderer if it was an exact match but if it
wasn't it just took the first renderer.

Instead, we can track the inner most node we've found so far. Then get
the ID from that node (which will be fast since it's now a perfect
match). This is a better match.

However, the main reason I'm doing this is because the old mechanism
leaked the `Fiber` type outside the `RendererInterface` which is
supposed to abstract all of that. With the new algorithm this doesn't
leak.

I've tested this with a new build against the repro in the old issue
#24539 and it seems to work.

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]

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(`

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);
     }

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,

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) => {

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,

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 + ')';
       }
     }
 

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);
       }