From 92219ff9b80d0884cfc9c4123e7dcbd54ec0221d Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Mon, 17 Jun 2024 16:13:04 +0100 Subject: [PATCH] chore[react-devtools/backend]: remove consoleManagedByDevToolsDuringStrictMode (#29856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Removes the usage of `consoleManagedByDevToolsDuringStrictMode` flag from React DevTools backend, this is the only place in RDT where this flag was used. The only remaining part is [`ReactFiberDevToolsHook`](https://github.com/facebook/react/blob/67081159377b438b48e3c2f2278af8e5f56b9f64/packages/react-reconciler/src/ReactFiberDevToolsHook.js#L203), so React renderers can start notifying DevTools when `render` runs in a Strict Mode. > TL;DR: it is broken, and we already incorrectly apply dimming, when RDT frontend is not opened. Fixing in the next few changes, see next steps. Before explaining why I am removing this, some context is required. The way RDT works is slightly different, based on the fact if RDT frontend and RDT backend are actually connected: 1. For browser extension case, the Backend is a script, which is injected by the extension when page is loaded and before React is loaded. RDT Frontend is loaded together with the RDT panel in browser DevTools, so ONLY when user actually opens the RDT panel. 2. For native case, RDT backend is shipped together with `react-native` for DEV bundles. It is always injected before React is loaded. RDT frontend is loaded only when user starts a standalone RDT app via `npx react-devtools` or by opening React Native DevTools and then selecting React DevTools panel. When Frontend is not connected to the Backend, the only thing we have is the `__REACT_DEVTOOLS_GLOBAL_HOOK__` — this thing inlines some APIs in itself, so that it can work similarly when RDT Frontend is not even opened. This is especially important for console logs, since they are cached and stored, then later displayed to the user once the Console panel is opened, but from RDT side, you want to modify these console logs when they are emitted. In order to do so, we [inline the console patching logic into the hook](https://github.com/facebook/react/blob/3ac551e855f9bec3161da2fc8787958aa62113db/packages/react-devtools-shared/src/hook.js#L222-L319). This implementation doesn't use the `consoleManagedByDevToolsDuringStrictMode`. This means that if we enable `consoleManagedByDevToolsDuringStrictMode` for Native right now, users would see broken dimming in LogBox / Metro logs when RDT Frontend is not opened. Next steps: 1. Align this console patching implementation with the one in `hook.js`. 2. Make LogBox compatible with console stylings: both css and ASCII escape symbols. 3. Ship new version of RDT with these changes. 4. Remove `consoleManagedByDevToolsDuringStrictMode` from `ReactFiberDevToolsHook`, so this is rolled out for all renderers. --- .../src/backend/console.js | 109 +++++++++--------- .../config/DevToolsFeatureFlags.core-fb.js | 1 - .../config/DevToolsFeatureFlags.core-oss.js | 1 - .../config/DevToolsFeatureFlags.default.js | 1 - .../DevToolsFeatureFlags.extension-fb.js | 1 - .../DevToolsFeatureFlags.extension-oss.js | 1 - 6 files changed, 52 insertions(+), 62 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 13c2249013..0aa0bcdbff 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -22,7 +22,6 @@ import { getStackByFiberInDevAndProd, supportsNativeConsoleTasks, } from './DevToolsFiberComponentStack'; -import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags'; import {castBool, castBrowserTheme} from '../utils'; const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn']; @@ -302,76 +301,72 @@ let unpatchForStrictModeFn: null | (() => void) = null; // NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialCommitInStrictMode export function patchForStrictMode() { - if (consoleManagedByDevToolsDuringStrictMode) { - const overrideConsoleMethods = [ - 'error', - 'group', - 'groupCollapsed', - 'info', - 'log', - 'trace', - 'warn', - ]; + const overrideConsoleMethods = [ + 'error', + 'group', + 'groupCollapsed', + 'info', + 'log', + 'trace', + 'warn', + ]; - if (unpatchForStrictModeFn !== null) { - // Don't patch twice. - return; - } + if (unpatchForStrictModeFn !== null) { + // Don't patch twice. + return; + } - const originalConsoleMethods: {[string]: $FlowFixMe} = {}; + const originalConsoleMethods: {[string]: $FlowFixMe} = {}; - unpatchForStrictModeFn = () => { - for (const method in originalConsoleMethods) { - try { - targetConsole[method] = originalConsoleMethods[method]; - } catch (error) {} - } - }; - - overrideConsoleMethods.forEach(method => { + unpatchForStrictModeFn = () => { + for (const method in originalConsoleMethods) { try { - const originalMethod = (originalConsoleMethods[method] = targetConsole[ - method - ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ - ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ - : targetConsole[method]); + targetConsole[method] = originalConsoleMethods[method]; + } catch (error) {} + } + }; - // $FlowFixMe[missing-local-annot] - const overrideMethod = (...args) => { - if (!consoleSettingsRef.hideConsoleLogsInStrictMode) { - // Dim the text color of the double logs if we're not - // hiding them. - if (isNode) { - originalMethod(DIMMED_NODE_CONSOLE_COLOR, format(...args)); + overrideConsoleMethods.forEach(method => { + try { + const originalMethod = (originalConsoleMethods[method] = targetConsole[ + method + ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ + : targetConsole[method]); + + // $FlowFixMe[missing-local-annot] + const overrideMethod = (...args) => { + if (!consoleSettingsRef.hideConsoleLogsInStrictMode) { + // Dim the text color of the double logs if we're not + // hiding them. + if (isNode) { + originalMethod(DIMMED_NODE_CONSOLE_COLOR, format(...args)); + } else { + const color = getConsoleColor(method); + if (color) { + originalMethod(...formatWithStyles(args, `color: ${color}`)); } else { - const color = getConsoleColor(method); - if (color) { - originalMethod(...formatWithStyles(args, `color: ${color}`)); - } else { - throw Error('Console color is not defined'); - } + throw Error('Console color is not defined'); } } - }; + } + }; - overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ = - originalMethod; - originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ = - overrideMethod; + overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ = + originalMethod; + originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ = + overrideMethod; - targetConsole[method] = overrideMethod; - } catch (error) {} - }); - } + targetConsole[method] = overrideMethod; + } catch (error) {} + }); } // NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialCommitInStrictMode export function unpatchForStrictMode(): void { - if (consoleManagedByDevToolsDuringStrictMode) { - if (unpatchForStrictModeFn !== null) { - unpatchForStrictModeFn(); - unpatchForStrictModeFn = null; - } + if (unpatchForStrictModeFn !== null) { + unpatchForStrictModeFn(); + unpatchForStrictModeFn = null; } } diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js index 9c29e361fa..47e15953c9 100644 --- a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js +++ b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js @@ -13,7 +13,6 @@ * It should always be imported from "react-devtools-feature-flags". ************************************************************************/ -export const consoleManagedByDevToolsDuringStrictMode = false; export const enableLogger = true; export const enableStyleXFeatures = true; export const isInternalFacebookBuild = true; diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js index 060e5e808e..e7ec62243a 100644 --- a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js +++ b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js @@ -13,7 +13,6 @@ * It should always be imported from "react-devtools-feature-flags". ************************************************************************/ -export const consoleManagedByDevToolsDuringStrictMode = false; export const enableLogger = false; export const enableStyleXFeatures = false; export const isInternalFacebookBuild = false; diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js index 15b764f8d3..bba2c8fcbf 100644 --- a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js +++ b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js @@ -13,7 +13,6 @@ * It should always be imported from "react-devtools-feature-flags". ************************************************************************/ -export const consoleManagedByDevToolsDuringStrictMode = true; export const enableLogger = false; export const enableStyleXFeatures = false; export const isInternalFacebookBuild = false; diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js index b25db375ef..55ea045715 100644 --- a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js +++ b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js @@ -13,7 +13,6 @@ * It should always be imported from "react-devtools-feature-flags". ************************************************************************/ -export const consoleManagedByDevToolsDuringStrictMode = true; export const enableLogger = true; export const enableStyleXFeatures = true; export const isInternalFacebookBuild = true; diff --git a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js index 144ddb301f..75c8f149b3 100644 --- a/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js +++ b/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js @@ -13,7 +13,6 @@ * It should always be imported from "react-devtools-feature-flags". ************************************************************************/ -export const consoleManagedByDevToolsDuringStrictMode = true; export const enableLogger = false; export const enableStyleXFeatures = false; export const isInternalFacebookBuild = false;