From bb6b86ed596399ddd8bf642404a9e68ae430a6ea Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 12 Sep 2024 13:59:29 +0100 Subject: [PATCH] refactor[react-devtools]: initialize renderer interface early (#30946) The current state is that `rendererInterface`, which contains all the backend logic, like generating component stack or attaching errors to fibers, or traversing the Fiber tree, ..., is only mounted after the Frontend is created. For browser extension, this means that we don't patch console or track errors and warnings before Chrome DevTools is opened. With these changes, `rendererInterface` is created right after `renderer` is injected from React via global hook object (e. g. `__REACT_DEVTOOLS_GLOBAL_HOOK__.inject(...)`. Because of the current implementation, in case of multiple Reacts on the page, all of them will patch the console independently. This will be fixed in one of the next PRs, where I am moving console patching to the global Hook. This change of course makes `hook.js` script bigger, but I think it is a reasonable trade-off for better DevX. We later can add more heuristics to optimize the performance (if necessary) of `rendererInterface` for cases when Frontend was connected late and Backend is attempting to flush out too many recorded operations. This essentially reverts https://github.com/facebook/react/pull/26563. --- .../dynamicallyInjectContentScripts.js | 8 -- .../src/contentScripts/renderer.js | 33 ------ .../webpack.config.js | 1 - .../src/attachRenderer.js | 61 +++++++++++ .../src/backend/agent.js | 15 ++- .../src/backend/index.js | 101 ++++-------------- .../src/backend/types.js | 1 + packages/react-devtools-shared/src/bridge.js | 3 +- .../src/devtools/store.js | 1 + packages/react-devtools-shared/src/hook.js | 20 ++-- 10 files changed, 110 insertions(+), 134 deletions(-) delete mode 100644 packages/react-devtools-extensions/src/contentScripts/renderer.js create mode 100644 packages/react-devtools-shared/src/attachRenderer.js diff --git a/packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js b/packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js index b1888b4e7cc59..9398d71a54e7c 100644 --- a/packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js +++ b/packages/react-devtools-extensions/src/background/dynamicallyInjectContentScripts.js @@ -25,14 +25,6 @@ const contentScriptsToInject = [ runAt: 'document_start', world: chrome.scripting.ExecutionWorld.MAIN, }, - { - id: '@react-devtools/renderer', - js: ['build/renderer.js'], - matches: [''], - persistAcrossSessions: true, - runAt: 'document_start', - world: chrome.scripting.ExecutionWorld.MAIN, - }, ]; async function dynamicallyInjectContentScripts() { diff --git a/packages/react-devtools-extensions/src/contentScripts/renderer.js b/packages/react-devtools-extensions/src/contentScripts/renderer.js deleted file mode 100644 index 361530334177c..0000000000000 --- a/packages/react-devtools-extensions/src/contentScripts/renderer.js +++ /dev/null @@ -1,33 +0,0 @@ -/** - * In order to support reload-and-profile functionality, the renderer needs to be injected before any other scripts. - * Since it is a complex file (with imports) we can't just toString() it like we do with the hook itself, - * So this entry point (one of the web_accessible_resources) provides a way to eagerly inject it. - * The hook will look for the presence of a global __REACT_DEVTOOLS_ATTACH__ and attach an injected renderer early. - * The normal case (not a reload-and-profile) will not make use of this entry point though. - * - * @flow - */ - -import {attach} from 'react-devtools-shared/src/backend/fiber/renderer'; -import {SESSION_STORAGE_RELOAD_AND_PROFILE_KEY} from 'react-devtools-shared/src/constants'; -import {sessionStorageGetItem} from 'react-devtools-shared/src/storage'; - -if ( - sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' && - !window.hasOwnProperty('__REACT_DEVTOOLS_ATTACH__') -) { - Object.defineProperty( - window, - '__REACT_DEVTOOLS_ATTACH__', - ({ - enumerable: false, - // This property needs to be configurable to allow third-party integrations - // to attach their own renderer. Note that using third-party integrations - // is not officially supported. Use at your own risk. - configurable: true, - get() { - return attach; - }, - }: Object), - ); -} diff --git a/packages/react-devtools-extensions/webpack.config.js b/packages/react-devtools-extensions/webpack.config.js index 81bf4a1c520b3..ddbb4356f658c 100644 --- a/packages/react-devtools-extensions/webpack.config.js +++ b/packages/react-devtools-extensions/webpack.config.js @@ -55,7 +55,6 @@ module.exports = { panel: './src/panel.js', proxy: './src/contentScripts/proxy.js', prepareInjection: './src/contentScripts/prepareInjection.js', - renderer: './src/contentScripts/renderer.js', installHook: './src/contentScripts/installHook.js', }, output: { diff --git a/packages/react-devtools-shared/src/attachRenderer.js b/packages/react-devtools-shared/src/attachRenderer.js new file mode 100644 index 0000000000000..3138f00cad615 --- /dev/null +++ b/packages/react-devtools-shared/src/attachRenderer.js @@ -0,0 +1,61 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type { + ReactRenderer, + RendererInterface, + DevToolsHook, + RendererID, +} from 'react-devtools-shared/src/backend/types'; + +import {attach as attachFlight} from 'react-devtools-shared/src/backend/flight/renderer'; +import {attach as attachFiber} from 'react-devtools-shared/src/backend/fiber/renderer'; +import {attach as attachLegacy} from 'react-devtools-shared/src/backend/legacy/renderer'; +import {hasAssignedBackend} from 'react-devtools-shared/src/backend/utils'; + +// this is the backend that is compatible with all older React versions +function isMatchingRender(version: string): boolean { + return !hasAssignedBackend(version); +} + +export default function attachRenderer( + hook: DevToolsHook, + id: RendererID, + renderer: ReactRenderer, + global: Object, +): RendererInterface | void { + // only attach if the renderer is compatible with the current version of the backend + if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) { + return; + } + let rendererInterface = hook.rendererInterfaces.get(id); + + // Inject any not-yet-injected renderers (if we didn't reload-and-profile) + if (rendererInterface == null) { + if (typeof renderer.getCurrentComponentInfo === 'function') { + // react-flight/client + rendererInterface = attachFlight(hook, id, renderer, global); + } else if ( + // v16-19 + typeof renderer.findFiberByHostInstance === 'function' || + // v16.8+ + renderer.currentDispatcherRef != null + ) { + // react-reconciler v16+ + rendererInterface = attachFiber(hook, id, renderer, global); + } else if (renderer.ComponentTree) { + // react-dom v15 + rendererInterface = attachLegacy(hook, id, renderer, global); + } else { + // Older react-dom or other unsupported renderer version + } + } + + return rendererInterface; +} diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 03ace4add2417..f4665e014c023 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -152,6 +152,7 @@ export default class Agent extends EventEmitter<{ traceUpdates: [Set], drawTraceUpdates: [Array], disableTraceUpdates: [], + getIfHasUnsupportedRendererVersion: [], }> { _bridge: BackendBridge; _isProfiling: boolean = false; @@ -221,6 +222,10 @@ export default class Agent extends EventEmitter<{ ); bridge.addListener('updateComponentFilters', this.updateComponentFilters); bridge.addListener('getEnvironmentNames', this.getEnvironmentNames); + bridge.addListener( + 'getIfHasUnsupportedRendererVersion', + this.getIfHasUnsupportedRendererVersion, + ); // Temporarily support older standalone front-ends sending commands to newer embedded backends. // We do this because React Native embeds the React DevTools backend, @@ -709,7 +714,7 @@ export default class Agent extends EventEmitter<{ } } - setRendererInterface( + registerRendererInterface( rendererID: RendererID, rendererInterface: RendererInterface, ) { @@ -940,8 +945,12 @@ export default class Agent extends EventEmitter<{ } }; - onUnsupportedRenderer(rendererID: number) { - this._bridge.send('unsupportedRendererVersion', rendererID); + getIfHasUnsupportedRendererVersion: () => void = () => { + this.emit('getIfHasUnsupportedRendererVersion'); + }; + + onUnsupportedRenderer() { + this._bridge.send('unsupportedRendererVersion'); } _persistSelectionTimerScheduled: boolean = false; diff --git a/packages/react-devtools-shared/src/backend/index.js b/packages/react-devtools-shared/src/backend/index.js index 943c0360ffc99..0ac7ecc468aa5 100644 --- a/packages/react-devtools-shared/src/backend/index.js +++ b/packages/react-devtools-shared/src/backend/index.js @@ -9,18 +9,7 @@ import Agent from './agent'; -import {attach as attachFiber} from './fiber/renderer'; -import {attach as attachFlight} from './flight/renderer'; -import {attach as attachLegacy} from './legacy/renderer'; - -import {hasAssignedBackend} from './utils'; - -import type {DevToolsHook, ReactRenderer, RendererInterface} from './types'; - -// this is the backend that is compatible with all older React versions -function isMatchingRender(version: string): boolean { - return !hasAssignedBackend(version); -} +import type {DevToolsHook, RendererID, RendererInterface} from './types'; export type InitBackend = typeof initBackend; @@ -34,29 +23,32 @@ export function initBackend( return () => {}; } + function registerRendererInterface( + id: RendererID, + rendererInterface: RendererInterface, + ) { + agent.registerRendererInterface(id, rendererInterface); + + // Now that the Store and the renderer interface are connected, + // it's time to flush the pending operation codes to the frontend. + rendererInterface.flushInitialOperations(); + } + const subs = [ hook.sub( 'renderer-attached', ({ id, - renderer, rendererInterface, }: { id: number, - renderer: ReactRenderer, rendererInterface: RendererInterface, - ... }) => { - agent.setRendererInterface(id, rendererInterface); - - // Now that the Store and the renderer interface are connected, - // it's time to flush the pending operation codes to the frontend. - rendererInterface.flushInitialOperations(); + registerRendererInterface(id, rendererInterface); }, ), - - hook.sub('unsupported-renderer-version', (id: number) => { - agent.onUnsupportedRenderer(id); + hook.sub('unsupported-renderer-version', () => { + agent.onUnsupportedRenderer(); }), hook.sub('fastRefreshScheduled', agent.onFastRefreshScheduled), @@ -66,68 +58,19 @@ export function initBackend( // TODO Add additional subscriptions required for profiling mode ]; - const attachRenderer = (id: number, renderer: ReactRenderer) => { - // only attach if the renderer is compatible with the current version of the backend - if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) { - return; - } - let rendererInterface = hook.rendererInterfaces.get(id); - - // Inject any not-yet-injected renderers (if we didn't reload-and-profile) - if (rendererInterface == null) { - if (typeof renderer.getCurrentComponentInfo === 'function') { - // react-flight/client - rendererInterface = attachFlight(hook, id, renderer, global); - } else if ( - // v16-19 - typeof renderer.findFiberByHostInstance === 'function' || - // v16.8+ - renderer.currentDispatcherRef != null - ) { - // react-reconciler v16+ - rendererInterface = attachFiber(hook, id, renderer, global); - } else if (renderer.ComponentTree) { - // react-dom v15 - rendererInterface = attachLegacy(hook, id, renderer, global); - } else { - // Older react-dom or other unsupported renderer version - } - - if (rendererInterface != null) { - hook.rendererInterfaces.set(id, rendererInterface); - } + agent.addListener('getIfHasUnsupportedRendererVersion', () => { + if (hook.hasUnsupportedRendererAttached) { + agent.onUnsupportedRenderer(); } - - // Notify the DevTools frontend about new renderers. - // This includes any that were attached early (via __REACT_DEVTOOLS_ATTACH__). - if (rendererInterface != null) { - hook.emit('renderer-attached', { - id, - renderer, - rendererInterface, - }); - } else { - hook.emit('unsupported-renderer-version', id); - } - }; - - // Connect renderers that have already injected themselves. - hook.renderers.forEach((renderer, id) => { - attachRenderer(id, renderer); }); - // Connect any new renderers that injected themselves. - subs.push( - hook.sub( - 'renderer', - ({id, renderer}: {id: number, renderer: ReactRenderer, ...}) => { - attachRenderer(id, renderer); - }, - ), - ); + hook.rendererInterfaces.forEach((rendererInterface, id) => { + registerRendererInterface(id, rendererInterface); + }); hook.emit('react-devtools', agent); hook.reactDevtoolsAgent = agent; + const onAgentShutdown = () => { subs.forEach(fn => fn()); hook.rendererInterfaces.forEach(rendererInterface => { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index dc3222072a482..e3c062fefaaf3 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -492,6 +492,7 @@ export type DevToolsHook = { listeners: {[key: string]: Array, ...}, rendererInterfaces: Map, renderers: Map, + hasUnsupportedRendererAttached: boolean, backends: Map, emit: (event: string, data: any) => void, diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 9e8bd71b71758..65a52b571a680 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -200,7 +200,7 @@ export type BackendEvents = { stopInspectingHost: [boolean], syncSelectionFromBuiltinElementsPanel: [], syncSelectionToBuiltinElementsPanel: [], - unsupportedRendererVersion: [RendererID], + unsupportedRendererVersion: [], // React Native style editor plug-in. isNativeStyleEditorSupported: [ @@ -218,6 +218,7 @@ type FrontendEvents = { deletePath: [DeletePath], getBackendVersion: [], getBridgeProtocol: [], + getIfHasUnsupportedRendererVersion: [], getOwnersList: [ElementAndRendererID], getProfilingData: [{rendererID: RendererID}], getProfilingStatus: [], diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index e4d4b9dcceec0..d351306a44546 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -1500,6 +1500,7 @@ export default class Store extends EventEmitter<{ } this._bridge.send('getBackendVersion'); + this._bridge.send('getIfHasUnsupportedRendererVersion'); }; // The Store should never throw an Error without also emitting an event. diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index aeef1abc0e737..eb81e81d4615e 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -21,6 +21,7 @@ import { FIREFOX_CONSOLE_DIMMING_COLOR, ANSI_STYLE_DIMMING_TEMPLATE, } from 'react-devtools-shared/src/constants'; +import attachRenderer from './attachRenderer'; declare var window: any; @@ -358,7 +359,6 @@ export function installHook(target: any): DevToolsHook | null { } let uidCounter = 0; - function inject(renderer: ReactRenderer): number { const id = ++uidCounter; renderers.set(id, renderer); @@ -367,20 +367,21 @@ export function installHook(target: any): DevToolsHook | null { ? 'deadcode' : detectReactBuildType(renderer); - // If we have just reloaded to profile, we need to inject the renderer interface before the app loads. - // Otherwise the renderer won't yet exist and we can skip this step. - const attach = target.__REACT_DEVTOOLS_ATTACH__; - if (typeof attach === 'function') { - const rendererInterface = attach(hook, id, renderer, target); - hook.rendererInterfaces.set(id, rendererInterface); - } - hook.emit('renderer', { id, renderer, reactBuildType, }); + const rendererInterface = attachRenderer(hook, id, renderer, target); + if (rendererInterface != null) { + hook.rendererInterfaces.set(id, rendererInterface); + hook.emit('renderer-attached', {id, rendererInterface}); + } else { + hook.hasUnsupportedRendererAttached = true; + hook.emit('unsupported-renderer-version'); + } + return id; } @@ -534,6 +535,7 @@ export function installHook(target: any): DevToolsHook | null { // Fast Refresh for web relies on this. renderers, + hasUnsupportedRendererAttached: false, emit, getFiberRoots,