[DevTools] find best renderer when inspecting (#24665)

* [DevTools] find best renderer when inspecting * fix lint * fix test * fix lint * move logic to agent * fix lint * style improvements per review comments * fix lint & flow * re-add try catch for safety
facebook · Jun 8, 2022 · 3e92eb0 · 3e92eb0
1 parent 42b330c
commit 3e92eb0
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 27 deletions.
diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
@@ -309,17 +309,31 @@ export default class Agent extends EventEmitter<{|
     return renderer.getInstanceAndStyle(id);
   }
 
-  getIDForNode(node: Object): number | null {
+  getBestMatchingRendererInterface(node: Object): RendererInterface | null {
+    let bestMatch = null;
     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;
+        }
+      }
+    }
+    // if an exact match is not found, return the first valid renderer as fallback
+    return bestMatch;
+  }
 
+  getIDForNode(node: Object): number | null {
+    const rendererInterface = this.getBestMatchingRendererInterface(node);
+    if (rendererInterface != null) {
       try {
-        const id = renderer.getFiberIDForNative(node, true);
-        if (id !== null) {
-          return id;
-        }
+        return rendererInterface.getFiberIDForNative(node, true);
       } catch (error) {
         // Some old React versions might throw if they can't find a match.
         // If so we should ignore it...

diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js
@@ -148,6 +148,10 @@ export function attach(
 
   let getInternalIDForNative: GetFiberIDForNative = ((null: any): GetFiberIDForNative);
   let findNativeNodeForInternalID: (id: number) => ?NativeType;
+  let getFiberForNative = (node: NativeType) => {
+    // Not implemented.
+    return null;
+  };
 
   if (renderer.ComponentTree) {
     getInternalIDForNative = (node, findNearestUnfilteredAncestor) => {
@@ -160,6 +164,9 @@ export function attach(
       const internalInstance = idToInternalInstanceMap.get(id);
       return renderer.ComponentTree.getNodeFromInstance(internalInstance);
     };
+    getFiberForNative = (node: NativeType) => {
+      return renderer.ComponentTree.getClosestInstanceFromNode(node);
+    };
   } else if (renderer.Mount.getID && renderer.Mount.getNode) {
     getInternalIDForNative = (node, findNearestUnfilteredAncestor) => {
       // Not implemented.
@@ -1094,6 +1101,7 @@ export function attach(
     flushInitialOperations,
     getBestMatchForTrackedPath,
     getDisplayNameForFiberID,
+    getFiberForNative,
     getFiberIDForNative: getInternalIDForNative,
     getInstanceAndStyle,
     findNativeNodesForFiberID: (id: number) => {

diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -2818,6 +2818,10 @@ export function attach(
     return fiber != null ? getDisplayNameForFiber(((fiber: any): Fiber)) : null;
   }
 
+  function getFiberForNative(hostInstance) {
+    return renderer.findFiberByHostInstance(hostInstance);
+  }
+
   function getFiberIDForNative(
     hostInstance,
     findNearestUnfilteredAncestor = false,
@@ -4490,6 +4494,7 @@ export function attach(
     flushInitialOperations,
     getBestMatchForTrackedPath,
     getDisplayNameForFiberID,
+    getFiberForNative,
     getFiberIDForNative,
     getInstanceAndStyle,
     getOwnersList,

diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -93,7 +93,7 @@ export type Lane = number;
 export type Lanes = number;
 
 export type ReactRenderer = {
-  findFiberByHostInstance: (hostInstance: NativeType) => ?Fiber,
+  findFiberByHostInstance: (hostInstance: NativeType) => Fiber | null,
   version: string,
   rendererPackageName: string,
   bundleType: BundleType,
@@ -350,6 +350,7 @@ export type RendererInterface = {
   findNativeNodesForFiberID: FindNativeNodesForFiberID,
   flushInitialOperations: () => void,
   getBestMatchForTrackedPath: () => PathMatch | null,
+  getFiberForNative: (component: NativeType) => Fiber | null,
   getFiberIDForNative: GetFiberIDForNative,
   getDisplayNameForFiberID: GetDisplayNameForFiberID,
   getInstanceAndStyle(id: number): InstanceAndStyle,

diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/Highlighter.js b/packages/react-devtools-shared/src/backend/views/Highlighter/Highlighter.js
@@ -7,6 +7,8 @@
  * @flow
  */
 
+import type Agent from 'react-devtools-shared/src/backend/agent';
+
 import Overlay from './Overlay';
 
 const SHOW_DURATION = 2000;
@@ -26,6 +28,7 @@ export function hideOverlay() {
 export function showOverlay(
   elements: Array<HTMLElement> | null,
   componentName: string | null,
+  agent: Agent,
   hideAfterTimeout: boolean,
 ) {
   // TODO (npm-packages) Detect RN and support it somehow
@@ -42,7 +45,7 @@ export function showOverlay(
   }
 
   if (overlay === null) {
-    overlay = new Overlay();
+    overlay = new Overlay(agent);
   }
 
   overlay.inspect(elements, componentName);

diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js b/packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js
@@ -9,13 +9,13 @@
 
 import {getElementDimensions, getNestedBoundingClientRect} from '../utils';
 
-const assign = Object.assign;
-
-import type {DevToolsHook} from 'react-devtools-shared/src/backend/types';
 import type {Rect} from '../utils';
+import type Agent from 'react-devtools-shared/src/backend/agent';
 
 type Box = {|top: number, left: number, width: number, height: number|};
 
+const assign = Object.assign;
+
 // Note that the Overlay components are not affected by the active Theme,
 // because they highlight elements in the main Chrome window (outside of devtools).
 // The colors below were chosen to roughly match those used by Chrome devtools.
@@ -153,8 +153,9 @@ export default class Overlay {
   container: HTMLElement;
   tip: OverlayTip;
   rects: Array<OverlayRect>;
+  agent: Agent;
 
-  constructor() {
+  constructor(agent: Agent) {
     // Find the root window, because overlays are positioned relative to it.
     const currentWindow = window.__REACT_DEVTOOLS_TARGET_WINDOW__ || window;
     this.window = currentWindow;
@@ -170,6 +171,8 @@ export default class Overlay {
     this.tip = new OverlayTip(doc, this.container);
     this.rects = [];
 
+    this.agent = agent;
+
     doc.body.appendChild(this.container);
   }
 
@@ -230,22 +233,20 @@ export default class Overlay {
       name = elements[0].nodeName.toLowerCase();
 
       const node = elements[0];
-      const hook: DevToolsHook =
-        node.ownerDocument.defaultView.__REACT_DEVTOOLS_GLOBAL_HOOK__;
-      if (hook != null && hook.rendererInterfaces != null) {
-        let ownerName = null;
-        // eslint-disable-next-line no-for-of-loops/no-for-of-loops
-        for (const rendererInterface of hook.rendererInterfaces.values()) {
-          const id = rendererInterface.getFiberIDForNative(node, true);
-          if (id !== null) {
-            ownerName = rendererInterface.getDisplayNameForFiberID(id, true);
-            break;
+      const rendererInterface = this.agent.getBestMatchingRendererInterface(
+        node,
+      );
+      if (rendererInterface) {
+        const id = rendererInterface.getFiberIDForNative(node, true);
+        if (id) {
+          const ownerName = rendererInterface.getDisplayNameForFiberID(
+            id,
+            true,
+          );
+          if (ownerName) {
+            name += ' (in ' + ownerName + ')';
           }
         }
-
-        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
@@ -118,7 +118,7 @@ export default function setupHighlighter(
         node.scrollIntoView({block: 'nearest', inline: 'nearest'});
       }
 
-      showOverlay(nodes, displayName, hideAfterTimeout);
+      showOverlay(nodes, displayName, agent, hideAfterTimeout);
 
       if (openNativeElementsPanel) {
         window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node;
@@ -171,7 +171,7 @@ export default function setupHighlighter(
 
     // Don't pass the name explicitly.
     // It will be inferred from DOM tag and Fiber owner.
-    showOverlay([target], null, false);
+    showOverlay([target], null, agent, false);
 
     selectFiberForNode(target);
   }