Skip to content

Commit 92219ff

Browse files
authored
chore[react-devtools/backend]: remove consoleManagedByDevToolsDuringStrictMode (facebook#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.
1 parent d7bb603 commit 92219ff

File tree

6 files changed

+53
-63
lines changed

6 files changed

+53
-63
lines changed

packages/react-devtools-shared/src/backend/console.js

Lines changed: 53 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
getStackByFiberInDevAndProd,
2323
supportsNativeConsoleTasks,
2424
} from './DevToolsFiberComponentStack';
25-
import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags';
2625
import {castBool, castBrowserTheme} from '../utils';
2726

2827
const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn'];
@@ -302,76 +301,72 @@ let unpatchForStrictModeFn: null | (() => void) = null;
302301

303302
// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialCommitInStrictMode
304303
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+
}
320318

321-
const originalConsoleMethods: {[string]: $FlowFixMe} = {};
319+
const originalConsoleMethods: {[string]: $FlowFixMe} = {};
322320

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) {
332323
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+
};
338328

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}`));
346348
} 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');
353350
}
354351
}
355-
};
352+
}
353+
};
356354

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

362-
targetConsole[method] = overrideMethod;
363-
} catch (error) {}
364-
});
365-
}
360+
targetConsole[method] = overrideMethod;
361+
} catch (error) {}
362+
});
366363
}
367364

368365
// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialCommitInStrictMode
369366
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;
375370
}
376371
}
377372

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-fb.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
export const consoleManagedByDevToolsDuringStrictMode = false;
1716
export const enableLogger = true;
1817
export const enableStyleXFeatures = true;
1918
export const isInternalFacebookBuild = true;

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.core-oss.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
export const consoleManagedByDevToolsDuringStrictMode = false;
1716
export const enableLogger = false;
1817
export const enableStyleXFeatures = false;
1918
export const isInternalFacebookBuild = false;

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.default.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
export const consoleManagedByDevToolsDuringStrictMode = true;
1716
export const enableLogger = false;
1817
export const enableStyleXFeatures = false;
1918
export const isInternalFacebookBuild = false;

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-fb.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
export const consoleManagedByDevToolsDuringStrictMode = true;
1716
export const enableLogger = true;
1817
export const enableStyleXFeatures = true;
1918
export const isInternalFacebookBuild = true;

packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* It should always be imported from "react-devtools-feature-flags".
1414
************************************************************************/
1515

16-
export const consoleManagedByDevToolsDuringStrictMode = true;
1716
export const enableLogger = false;
1817
export const enableStyleXFeatures = false;
1918
export const isInternalFacebookBuild = false;

0 commit comments

Comments
 (0)