From 3f4e8f287858e529aeb62d28032877e803ed15e7 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Fri, 7 Apr 2023 03:35:36 -0400 Subject: [PATCH] [DevTools] remove backend dependency from the global hook (#26563) ## Summary - #26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js --- packages/react-devtools-core/src/backend.js | 4 ++ .../chrome/manifest.json | 1 + .../edge/manifest.json | 1 + .../firefox/manifest.json | 1 + .../src/background.js | 7 +++ .../src/contentScripts/prepareInjection.js | 12 ++++- .../src/contentScripts/renderer.js | 33 ++++++++++++ .../webpack.config.js | 1 + packages/react-devtools-inline/src/backend.js | 4 ++ .../src/backend/console.js | 7 +++ .../src/backend/index.js | 3 +- packages/react-devtools-shared/src/hook.js | 51 ++++++------------- 12 files changed, 85 insertions(+), 40 deletions(-) create mode 100644 packages/react-devtools-extensions/src/contentScripts/renderer.js diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js index 27faa9f15db2e..d4db634f94e97 100644 --- a/packages/react-devtools-core/src/backend.js +++ b/packages/react-devtools-core/src/backend.js @@ -11,6 +11,7 @@ import Agent from 'react-devtools-shared/src/backend/agent'; import Bridge from 'react-devtools-shared/src/bridge'; import {installHook} from 'react-devtools-shared/src/hook'; import {initBackend} from 'react-devtools-shared/src/backend'; +import {installConsoleFunctionsToWindow} from 'react-devtools-shared/src/backend/console'; import {__DEBUG__} from 'react-devtools-shared/src/constants'; import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor'; import {getDefaultComponentFilters} from 'react-devtools-shared/src/utils'; @@ -38,6 +39,9 @@ type ConnectOptions = { ... }; +// Install a global variable to allow patching console early (during injection). +// This provides React Native developers with components stacks even if they don't run DevTools. +installConsoleFunctionsToWindow(); installHook(window); const hook: ?DevToolsHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__; diff --git a/packages/react-devtools-extensions/chrome/manifest.json b/packages/react-devtools-extensions/chrome/manifest.json index 32abe1ffaa3c0..d27120df284e9 100644 --- a/packages/react-devtools-extensions/chrome/manifest.json +++ b/packages/react-devtools-extensions/chrome/manifest.json @@ -31,6 +31,7 @@ "panel.html", "build/react_devtools_backend.js", "build/proxy.js", + "build/renderer.js", "build/installHook.js" ], "matches": [ diff --git a/packages/react-devtools-extensions/edge/manifest.json b/packages/react-devtools-extensions/edge/manifest.json index 23b535099943f..e0699d40d00ea 100644 --- a/packages/react-devtools-extensions/edge/manifest.json +++ b/packages/react-devtools-extensions/edge/manifest.json @@ -31,6 +31,7 @@ "panel.html", "build/react_devtools_backend.js", "build/proxy.js", + "build/renderer.js", "build/installHook.js" ], "matches": [ diff --git a/packages/react-devtools-extensions/firefox/manifest.json b/packages/react-devtools-extensions/firefox/manifest.json index 0b78ab196cb98..e0680a617ca3f 100644 --- a/packages/react-devtools-extensions/firefox/manifest.json +++ b/packages/react-devtools-extensions/firefox/manifest.json @@ -32,6 +32,7 @@ "panel.html", "build/react_devtools_backend.js", "build/proxy.js", + "build/renderer.js", "build/installHook.js" ], "background": { diff --git a/packages/react-devtools-extensions/src/background.js b/packages/react-devtools-extensions/src/background.js index bab24ddf5fe5c..fea54f292c0d4 100644 --- a/packages/react-devtools-extensions/src/background.js +++ b/packages/react-devtools-extensions/src/background.js @@ -22,6 +22,13 @@ if (!IS_FIREFOX) { runAt: 'document_start', world: chrome.scripting.ExecutionWorld.MAIN, }, + { + id: 'renderer', + matches: [''], + js: ['build/renderer.js'], + runAt: 'document_start', + world: chrome.scripting.ExecutionWorld.MAIN, + }, ], function () { // When the content scripts are already registered, an error will be thrown. diff --git a/packages/react-devtools-extensions/src/contentScripts/prepareInjection.js b/packages/react-devtools-extensions/src/contentScripts/prepareInjection.js index 7c136aee58b67..bbba91855db86 100644 --- a/packages/react-devtools-extensions/src/contentScripts/prepareInjection.js +++ b/packages/react-devtools-extensions/src/contentScripts/prepareInjection.js @@ -1,6 +1,8 @@ /* global chrome */ import nullthrows from 'nullthrows'; +import {SESSION_STORAGE_RELOAD_AND_PROFILE_KEY} from 'react-devtools-shared/src/constants'; +import {sessionStorageGetItem} from 'react-devtools-shared/src/storage'; import {IS_FIREFOX} from '../utils'; // We run scripts on the page via the service worker (backgroud.js) for @@ -109,9 +111,15 @@ window.addEventListener('pageshow', function ({target}) { chrome.runtime.sendMessage(lastDetectionResult); }); -// Inject a __REACT_DEVTOOLS_GLOBAL_HOOK__ global for React to interact with. -// Only do this for HTML documents though, to avoid e.g. breaking syntax highlighting for XML docs. if (IS_FIREFOX) { + // If we have just reloaded to profile, we need to inject the renderer interface before the app loads. + if ( + sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' + ) { + injectScriptSync(chrome.runtime.getURL('build/renderer.js')); + } + // Inject a __REACT_DEVTOOLS_GLOBAL_HOOK__ global for React to interact with. + // Only do this for HTML documents though, to avoid e.g. breaking syntax highlighting for XML docs. switch (document.contentType) { case 'text/html': case 'application/xhtml+xml': { diff --git a/packages/react-devtools-extensions/src/contentScripts/renderer.js b/packages/react-devtools-extensions/src/contentScripts/renderer.js new file mode 100644 index 0000000000000..ab02997bbaab8 --- /dev/null +++ b/packages/react-devtools-extensions/src/contentScripts/renderer.js @@ -0,0 +1,33 @@ +/** + * 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/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 541f33466ebd6..fe5543c04b524 100644 --- a/packages/react-devtools-extensions/webpack.config.js +++ b/packages/react-devtools-extensions/webpack.config.js @@ -55,6 +55,7 @@ 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-inline/src/backend.js b/packages/react-devtools-inline/src/backend.js index eb70e9d15b811..c647eccaf1773 100644 --- a/packages/react-devtools-inline/src/backend.js +++ b/packages/react-devtools-inline/src/backend.js @@ -3,6 +3,7 @@ import Agent from 'react-devtools-shared/src/backend/agent'; import Bridge from 'react-devtools-shared/src/bridge'; import {initBackend} from 'react-devtools-shared/src/backend'; +import {installConsoleFunctionsToWindow} from 'react-devtools-shared/src/backend/console'; import {installHook} from 'react-devtools-shared/src/hook'; import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor'; @@ -119,5 +120,8 @@ export function createBridge(contentWindow: any, wall?: Wall): BackendBridge { } export function initialize(contentWindow: any): void { + // Install a global variable to allow patching console early (during injection). + // This provides React Native developers with components stacks even if they don't run DevTools. + installConsoleFunctionsToWindow(); installHook(contentWindow); } diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 2eb6f90c64164..27ca30114c090 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -413,3 +413,10 @@ export function writeConsolePatchSettingsToWindow( settings.hideConsoleLogsInStrictMode; window.__REACT_DEVTOOLS_BROWSER_THEME__ = settings.browserTheme; } + +export function installConsoleFunctionsToWindow(): void { + window.__REACT_DEVTOOLS_CONSOLE_FUNCTIONS__ = { + patchConsoleUsingWindowValues, + registerRendererWithConsole: registerRenderer, + }; +} diff --git a/packages/react-devtools-shared/src/backend/index.js b/packages/react-devtools-shared/src/backend/index.js index 3a0ff3befa0be..6aa8a2f58a300 100644 --- a/packages/react-devtools-shared/src/backend/index.js +++ b/packages/react-devtools-shared/src/backend/index.js @@ -76,8 +76,7 @@ export function initBackend( } // Notify the DevTools frontend about new renderers. - // This includes any that were attached early - // (when SESSION_STORAGE_RELOAD_AND_PROFILE_KEY is set to true). + // This includes any that were attached early (via __REACT_DEVTOOLS_ATTACH__). if (rendererInterface != null) { hook.emit('renderer-attached', { id, diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index d0ce3b7d8f200..09e7843b71faa 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -1,8 +1,8 @@ /** * Install the hook on window, which is an event emitter. - * Note because Chrome content scripts cannot directly modify the window object, - * we are evaling this function by inserting a script tag. - * That's why we have to inline the whole event emitter implementation, + * Note: this global hook __REACT_DEVTOOLS_GLOBAL_HOOK__ is a de facto public API. + * It's especially important to avoid creating direct dependency on the DevTools Backend. + * That's why we still inline the whole event emitter implementation, * the string format implementation, and part of the console implementation here. * * @flow @@ -17,14 +17,6 @@ import type { RendererInterface, } from './backend/types'; -import { - patchConsoleUsingWindowValues, - registerRenderer as registerRendererWithConsole, -} from './backend/console'; -import {attach} from './backend/renderer'; -import {SESSION_STORAGE_RELOAD_AND_PROFILE_KEY} from './constants'; -import {sessionStorageGetItem} from './storage'; - declare var window: any; export function installHook(target: any): DevToolsHook | null { @@ -340,37 +332,24 @@ export function installHook(target: any): DevToolsHook | null { // * Disabling or marking logs during a double render in Strict Mode // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber) // - // For React Native, we intentionally patch early (during injection). - // This provides React Native developers with components stacks even if they don't run DevTools. - // - // This won't work for DOM though, since this entire file is eval'ed and inserted as a script tag. - // In that case, we'll only patch parts of the console that are needed during the first render - // and patch everything else later (when the frontend attaches). - // - // Don't patch in test environments because we don't want to interfere with Jest's own console overrides. - // - // Note that because this function is inlined, this conditional check must only use static booleans. - // Otherwise the extension will throw with an undefined error. - // (See comments in the try/catch below for more context on inlining.) - if (!__TEST__ && !__EXTENSION__) { - try { - // The installHook() function is injected by being stringified in the browser, - // so imports outside of this function do not get included. - // - // Normally we could check "typeof patchConsole === 'function'", - // but Webpack wraps imports with an object (e.g. _backend_console__WEBPACK_IMPORTED_MODULE_0__) - // and the object itself will be undefined as well for the reasons mentioned above, - // so we use try/catch instead. + // Allow patching console early (during injection) to + // provide developers with components stacks even if they don't run DevTools. + if (target.hasOwnProperty('__REACT_DEVTOOLS_CONSOLE_FUNCTIONS__')) { + const {registerRendererWithConsole, patchConsoleUsingWindowValues} = + target.__REACT_DEVTOOLS_CONSOLE_FUNCTIONS__; + if ( + typeof registerRendererWithConsole === 'function' && + typeof patchConsoleUsingWindowValues === 'function' + ) { registerRendererWithConsole(renderer); patchConsoleUsingWindowValues(); - } catch (error) {} + } } // 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. - if ( - sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' - ) { + const attach = target.__REACT_DEVTOOLS_ATTACH__; + if (typeof attach === 'function') { const rendererInterface = attach(hook, id, renderer, target); hook.rendererInterfaces.set(id, rendererInterface); }