From f65ac7bd4aac61db1ec25af5b03b72d03779a890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 26 Aug 2024 11:53:17 -0400 Subject: [PATCH] [DevTools] Make function inspection instant (#30786) 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. --- .../src/main/index.js | 16 +---- .../src/main/sourceSelection.js | 59 +++++++++++++++++++ .../src/backend/agent.js | 20 ------- .../src/backend/fiber/renderer.js | 34 +++++------ .../src/backend/legacy/renderer.js | 19 +++--- .../src/backend/types.js | 6 +- 6 files changed, 90 insertions(+), 64 deletions(-) create mode 100644 packages/react-devtools-extensions/src/main/sourceSelection.js diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index d0bc285b11287..3a51b996e2049 100644 --- 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 new file mode 100644 index 0000000000000..0534a921af05e --- /dev/null +++ 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 index e81af56ebdf76..a71b259441e98 100644 --- 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) => 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 index 62eacdf5dbe9a..57eafa176f550 100644 --- 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, - ): 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 index 1955465607d2c..f8aa548a0573a 100644 --- 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, - ): 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 index 2bd13a3a1294d..87b0f2048b9db 100644 --- 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, - ) => void, - prepareViewElementSource: (id: number) => void, + ) => mixed, + getElementSourceFunctionById: (id: number) => null | Function, renamePath: ( type: Type, id: number,