@samitouri / QOS-React / commits / 92219ff9b8

chore[react-devtools/backend]: remove consoleManagedByDevToolsDuringStrictMode (#29856)

## 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.

Ruslan Lesiutin committed Jun 17, 2024 at 16:13 UTC 92219ff9b80d0884cfc9c4123e7dcbd54ec0221d
6 files changed +53 -63
packages/react-devtools-shared/src/backend/console.js
+53 -58
@@ -22,7 +22,6 @@ import {
22 getStackByFiberInDevAndProd,
23 supportsNativeConsoleTasks,
24 } from './DevToolsFiberComponentStack';
25 -import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags';
25 import {castBool, castBrowserTheme} from '../utils';
26
27 const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn'];
@@ -302,76 +301,72 @@ let unpatchForStrictModeFn: null | (() => void) = null;
301
302 // NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialCommitInStrictMode
303 export function patchForStrictMode() {
305 - if (consoleManagedByDevToolsDuringStrictMode) {
306 - const overrideConsoleMethods = [
307 - 'error',
308 - 'group',
309 - 'groupCollapsed',
310 - 'info',
311 - 'log',
312 - 'trace',
313 - 'warn',
314 - ];
315 -
316 - if (unpatchForStrictModeFn !== null) {
317 - // Don't patch twice.
318 - return;
319 - }
304 + const overrideConsoleMethods = [
305 + 'error',
306 + 'group',
307 + 'groupCollapsed',
308 + 'info',
309 + 'log',
310 + 'trace',
311 + 'warn',
312 + ];
313 +
314 + if (unpatchForStrictModeFn !== null) {
315 + // Don't patch twice.
316 + return;
317 + }
318
321 - const originalConsoleMethods: {[string]: $FlowFixMe} = {};
319 + const originalConsoleMethods: {[string]: $FlowFixMe} = {};
320
323 - unpatchForStrictModeFn = () => {
324 - for (const method in originalConsoleMethods) {
325 - try {
326 - targetConsole[method] = originalConsoleMethods[method];
327 - } catch (error) {}
328 - }
329 - };
330 -
331 - overrideConsoleMethods.forEach(method => {
321 + unpatchForStrictModeFn = () => {
322 + for (const method in originalConsoleMethods) {
323 try {
333 - const originalMethod = (originalConsoleMethods[method] = targetConsole[
334 - method
335 - ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__
336 - ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__
337 - : targetConsole[method]);
324 + targetConsole[method] = originalConsoleMethods[method];
325 + } catch (error) {}
326 + }
327 + };
328
339 - // $FlowFixMe[missing-local-annot]
340 - const overrideMethod = (...args) => {
341 - if (!consoleSettingsRef.hideConsoleLogsInStrictMode) {
342 - // Dim the text color of the double logs if we're not
343 - // hiding them.
344 - if (isNode) {
345 - originalMethod(DIMMED_NODE_CONSOLE_COLOR, format(...args));
329 + overrideConsoleMethods.forEach(method => {
330 + try {
331 + const originalMethod = (originalConsoleMethods[method] = targetConsole[
332 + method
333 + ].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__
334 + ? targetConsole[method].__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__
335 + : targetConsole[method]);
336 +
337 + // $FlowFixMe[missing-local-annot]
338 + const overrideMethod = (...args) => {
339 + if (!consoleSettingsRef.hideConsoleLogsInStrictMode) {
340 + // Dim the text color of the double logs if we're not
341 + // hiding them.
342 + if (isNode) {
343 + originalMethod(DIMMED_NODE_CONSOLE_COLOR, format(...args));
344 + } else {
345 + const color = getConsoleColor(method);
346 + if (color) {
347 + originalMethod(...formatWithStyles(args, `color: ${color}`));
348 } else {
347 - const color = getConsoleColor(method);
348 - if (color) {
349 - originalMethod(...formatWithStyles(args, `color: ${color}`));
350 - } else {
351 - throw Error('Console color is not defined');
352 - }
349 + throw Error('Console color is not defined');
350 }
351 }
355 - };
352 + }
353 + };
354
357 - overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ =
358 - originalMethod;
359 - originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ =
360 - overrideMethod;
355 + overrideMethod.__REACT_DEVTOOLS_STRICT_MODE_ORIGINAL_METHOD__ =
356 + originalMethod;
357 + originalMethod.__REACT_DEVTOOLS_STRICT_MODE_OVERRIDE_METHOD__ =
358 + overrideMethod;
359
362 - targetConsole[method] = overrideMethod;
363 - } catch (error) {}
364 - });
365 - }
360 + targetConsole[method] = overrideMethod;
361 + } catch (error) {}
362 + });
363 }
364
365 // NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialCommitInStrictMode
366 export function unpatchForStrictMode(): void {
370 - if (consoleManagedByDevToolsDuringStrictMode) {
371 - if (unpatchForStrictModeFn !== null) {
372 - unpatchForStrictModeFn();
373 - unpatchForStrictModeFn = null;
374 - }
367 + if (unpatchForStrictModeFn !== null) {
368 + unpatchForStrictModeFn();
369 + unpatchForStrictModeFn = null;
370 }
371 }
372
packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js
-1
@@ -13,7 +13,6 @@
13 * It should always be imported from "react-devtools-feature-flags".
14 ************************************************************************/
15
16 -export const consoleManagedByDevToolsDuringStrictMode = false;
16 export const enableLogger = true;
17 export const enableStyleXFeatures = true;
18 export const isInternalFacebookBuild = true;
packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js
-1
@@ -13,7 +13,6 @@
13 * It should always be imported from "react-devtools-feature-flags".
14 ************************************************************************/
15
16 -export const consoleManagedByDevToolsDuringStrictMode = false;
16 export const enableLogger = false;
17 export const enableStyleXFeatures = false;
18 export const isInternalFacebookBuild = false;
packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js
-1
@@ -13,7 +13,6 @@
13 * It should always be imported from "react-devtools-feature-flags".
14 ************************************************************************/
15
16 -export const consoleManagedByDevToolsDuringStrictMode = true;
16 export const enableLogger = false;
17 export const enableStyleXFeatures = false;
18 export const isInternalFacebookBuild = false;
packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js
-1
@@ -13,7 +13,6 @@
13 * It should always be imported from "react-devtools-feature-flags".
14 ************************************************************************/
15
16 -export const consoleManagedByDevToolsDuringStrictMode = true;
16 export const enableLogger = true;
17 export const enableStyleXFeatures = true;
18 export const isInternalFacebookBuild = true;
packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js
-1
@@ -13,7 +13,6 @@
13 * It should always be imported from "react-devtools-feature-flags".
14 ************************************************************************/
15
16 -export const consoleManagedByDevToolsDuringStrictMode = true;
16 export const enableLogger = false;
17 export const enableStyleXFeatures = false;
18 export const isInternalFacebookBuild = false;