Skip to content

Commit

Permalink
chore[react-devtools/backend]: remove consoleManagedByDevToolsDuringS…
Browse files Browse the repository at this point in the history
…trictMode (#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.
  • Loading branch information
hoxyq authored Jun 17, 2024
1 parent d7bb603 commit 92219ff
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 63 deletions.
111 changes: 53 additions & 58 deletions packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -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',
];

if (unpatchForStrictModeFn !== null) {
// Don't patch twice.
return;
}
const overrideConsoleMethods = [
'error',
'group',
'groupCollapsed',
'info',
'log',
'trace',
'warn',
];

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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 92219ff

Please sign in to comment.