[DevTools] Make function inspection instant (#30786)

sebmarkbage · web-flow · commit f65ac7bd4aac · 2024-08-26T11:53:17.000-04:00
I noticed that there is a delay due to the inspection being split into one part that gets the attribute and another eval that does the inspection. This is a bit hacky and uses temporary global names that are leaky. The timeout was presumably to ensure that the first step had fully propagated but it's slow. As we've learned, it can be throttled, and it isn't a guarantee either way. Instead, we can just consolidate these into a single operation that by-passes the bridge and goes straight to the renderer interface from the eval. I did the same for the viewElementSource helper even though that's not currently in use since #28471 but I think we probably should return to that technique when it's available since it's more reliable than the throw - at least in Chrome. I'm not sure about the status of React Native here. In Firefox, inspecting a function with source maps doesn't seem to work. It doesn't jump to original code.
diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js
@@ -21,6 +21,8 @@ import {
   setBrowserSelectionFromReact,
   setReactSelectionFromBrowser,
 } from './elementSelection';
+import {viewAttributeSource} from './sourceSelection';
+
 import {startReactPolling} from './reactPolling';
 import cloneStyleTags from './cloneStyleTags';
 import fetchFileWithCaching from './fetchFileWithCaching';
@@ -113,19 +115,7 @@ function createBridgeAndStore() {
   const viewAttributeSourceFunction = (id, path) => {
     const rendererID = store.getRendererIDForElement(id);
     if (rendererID != null) {
-      // Ask the renderer interface to find the specified attribute,
-      // and store it as a global variable on the window.
-      bridge.send('viewAttributeSource', {id, path, rendererID});
-
-      setTimeout(() => {
-        // Ask Chrome to display the location of the attribute,
-        // assuming the renderer found a match.
-        chrome.devtools.inspectedWindow.eval(`
-                if (window.$attribute != null) {
-                  inspect(window.$attribute);
-                }
-              `);
-      }, 100);
+      viewAttributeSource(rendererID, id, path);
     }
   };
 
diff --git a/packages/react-devtools-extensions/src/main/sourceSelection.js b/packages/react-devtools-extensions/src/main/sourceSelection.js
@@ -0,0 +1,59 @@
+/* global chrome */
+
+export function viewAttributeSource(rendererID, elementID, path) {
+  chrome.devtools.inspectedWindow.eval(
+    '{' + // The outer block is important because it means we can declare local variables.
+      'const renderer = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.get(' +
+      JSON.stringify(rendererID) +
+      ');' +
+      'if (renderer) {' +
+      '  const value = renderer.getElementAttributeByPath(' +
+      JSON.stringify(elementID) +
+      ',' +
+      JSON.stringify(path) +
+      ');' +
+      '  if (value) {' +
+      '    inspect(value);' +
+      '    true;' +
+      '  } else {' +
+      '    false;' +
+      '  }' +
+      '} else {' +
+      '  false;' +
+      '}' +
+      '}',
+    (didInspect, evalError) => {
+      if (evalError) {
+        console.error(evalError);
+      }
+    },
+  );
+}
+
+export function viewElementSource(rendererID, elementID) {
+  chrome.devtools.inspectedWindow.eval(
+    '{' + // The outer block is important because it means we can declare local variables.
+      'const renderer = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.get(' +
+      JSON.stringify(rendererID) +
+      ');' +
+      'if (renderer) {' +
+      '  const value = renderer.getElementSourceFunctionById(' +
+      JSON.stringify(elementID) +
+      ');' +
+      '  if (value) {' +
+      '    inspect(value);' +
+      '    true;' +
+      '  } else {' +
+      '    false;' +
+      '  }' +
+      '} else {' +
+      '  false;' +
+      '}' +
+      '}',
+    (didInspect, evalError) => {
+      if (evalError) {
+        console.error(evalError);
+      }
+    },
+  );
+}
diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
@@ -220,8 +220,6 @@ export default class Agent extends EventEmitter<{
       this.updateConsolePatchSettings,
     );
     bridge.addListener('updateComponentFilters', this.updateComponentFilters);
-    bridge.addListener('viewAttributeSource', this.viewAttributeSource);
-    bridge.addListener('viewElementSource', this.viewElementSource);
 
     // Temporarily support older standalone front-ends sending commands to newer embedded backends.
     // We do this because React Native embeds the React DevTools backend,
@@ -816,24 +814,6 @@ export default class Agent extends EventEmitter<{
       }
     };
 
-  viewAttributeSource: CopyElementParams => void = ({id, path, rendererID}) => {
-    const renderer = this._rendererInterfaces[rendererID];
-    if (renderer == null) {
-      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
-    } else {
-      renderer.prepareViewAttributeSource(id, path);
-    }
-  };
-
-  viewElementSource: ElementAndRendererID => void = ({id, rendererID}) => {
-    const renderer = this._rendererInterfaces[rendererID];
-    if (renderer == null) {
-      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
-    } else {
-      renderer.prepareViewElementSource(id);
-    }
-  };
-
   onTraceUpdates: (nodes: Set<HostInstance>) => void = nodes => {
     this.emit('traceUpdates', nodes);
   };
diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -3874,27 +3874,28 @@ export function attach(
 
   // END copied code
 
-  function prepareViewAttributeSource(
+  function getElementAttributeByPath(
     id: number,
     path: Array<string | number>,
-  ): void {
+  ): mixed {
     if (isMostRecentlyInspectedElement(id)) {
-      window.$attribute = getInObject(
+      return getInObject(
         ((mostRecentlyInspectedElement: any): InspectedElement),
         path,
       );
     }
+    return undefined;
   }
 
-  function prepareViewElementSource(id: number): void {
+  function getElementSourceFunctionById(id: number): null | Function {
     const devtoolsInstance = idToDevToolsInstanceMap.get(id);
     if (devtoolsInstance === undefined) {
       console.warn(`Could not find DevToolsInstance with id "${id}"`);
-      return;
+      return null;
     }
     if (devtoolsInstance.kind !== FIBER_INSTANCE) {
       // TODO: Handle VirtualInstance.
-      return;
+      return null;
     }
     const fiber = devtoolsInstance.data;
 
@@ -3906,21 +3907,16 @@ export function attach(
       case IncompleteFunctionComponent:
       case IndeterminateComponent:
       case FunctionComponent:
-        global.$type = type;
-        break;
+        return type;
       case ForwardRef:
-        global.$type = type.render;
-        break;
+        return type.render;
       case MemoComponent:
       case SimpleMemoComponent:
-        global.$type =
-          elementType != null && elementType.type != null
-            ? elementType.type
-            : type;
-        break;
+        return elementType != null && elementType.type != null
+          ? elementType.type
+          : type;
       default:
-        global.$type = null;
-        break;
+        return null;
     }
   }
 
@@ -5727,8 +5723,8 @@ export function attach(
     inspectElement,
     logElementToConsole,
     patchConsoleForStrictMode,
-    prepareViewAttributeSource,
-    prepareViewElementSource,
+    getElementAttributeByPath,
+    getElementSourceFunctionById,
     overrideError,
     overrideSuspense,
     overrideValueAtPath,
diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js
@@ -907,30 +907,31 @@ export function attach(
     }
   }
 
-  function prepareViewAttributeSource(
+  function getElementAttributeByPath(
     id: number,
     path: Array<string | number>,
-  ): void {
+  ): mixed {
     const inspectedElement = inspectElementRaw(id);
     if (inspectedElement !== null) {
-      window.$attribute = getInObject(inspectedElement, path);
+      return getInObject(inspectedElement, path);
     }
+    return undefined;
   }
 
-  function prepareViewElementSource(id: number): void {
+  function getElementSourceFunctionById(id: number): null | Function {
     const internalInstance = idToInternalInstanceMap.get(id);
     if (internalInstance == null) {
       console.warn(`Could not find instance with id "${id}"`);
-      return;
+      return null;
     }
 
     const element = internalInstance._currentElement;
     if (element == null) {
       console.warn(`Could not find element with id "${id}"`);
-      return;
+      return null;
     }
 
-    global.$type = element.type;
+    return element.type;
   }
 
   function deletePath(
@@ -1141,8 +1142,8 @@ export function attach(
     overrideValueAtPath,
     renamePath,
     patchConsoleForStrictMode,
-    prepareViewAttributeSource,
-    prepareViewElementSource,
+    getElementAttributeByPath,
+    getElementSourceFunctionById,
     renderer,
     setTraceUpdatesEnabled,
     setTrackedPath,
diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -394,11 +394,11 @@ export type RendererInterface = {
     value: any,
   ) => void,
   patchConsoleForStrictMode: () => void,
-  prepareViewAttributeSource: (
+  getElementAttributeByPath: (
     id: number,
     path: Array<string | number>,
-  ) => void,
-  prepareViewElementSource: (id: number) => void,
+  ) => mixed,
+  getElementSourceFunctionById: (id: number) => null | Function,
   renamePath: (
     type: Type,
     id: number,
