Skip to content

Commit 48f30ef

Browse files
Brian Vaughnkoto
authored andcommitted
DevTools fork console patching logic (facebook#21301)
React has its own component stack generation code that DevTools embeds a fork of, but both of them use a shared helper for disabling console logs. This shared helper is DEV only though, because it was intended for use with React DEV-only warnings and we didn't want to unnecessarily add bytes to production builds. But DevTools itself always ships as a production build– even when it's used to debug DEV bundles of product apps (with third party DEV-only warnings). That means this helper was always a noop. The resolveCurrentDispatcher method was changed recently to replace the thrown error with a call to console.error. This newly logged error ended up slipping through and being user visible because of the above issue. This PR updates DevTools to also fork the console patching logic (to remove the DEV-only guard). Note that I didn't spot this earlier because my test harness (react-devtools-shell) always runs in DEV mode. 🤡
1 parent 473224e commit 48f30ef

File tree

3 files changed

+96
-3
lines changed

3 files changed

+96
-3
lines changed

packages/react-devtools-core/webpack.backend.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ module.exports = {
5858
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
5959
}),
6060
],
61+
optimization: {
62+
minimize: false,
63+
},
6164
module: {
6265
rules: [
6366
{

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ import {
2929
SUSPENSE_LIST_SYMBOL_STRING,
3030
} from './ReactSymbols';
3131

32-
// These methods are safe to import from shared;
33-
// there is no React-specific logic here.
34-
import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev';
32+
// The shared console patching code is DEV-only.
33+
// We can't use it since DevTools only ships production builds.
34+
import {disableLogs, reenableLogs} from './DevToolsConsolePatching';
3535

3636
let prefix;
3737
export function describeBuiltInComponentFrame(
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
// This is a DevTools fork of shared/ConsolePatchingDev.
11+
// The shared console patching code is DEV-only.
12+
// We can't use it since DevTools only ships production builds.
13+
14+
// Helpers to patch console.logs to avoid logging during side-effect free
15+
// replaying on render function. This currently only patches the object
16+
// lazily which won't cover if the log function was extracted eagerly.
17+
// We could also eagerly patch the method.
18+
19+
let disabledDepth = 0;
20+
let prevLog;
21+
let prevInfo;
22+
let prevWarn;
23+
let prevError;
24+
let prevGroup;
25+
let prevGroupCollapsed;
26+
let prevGroupEnd;
27+
28+
function disabledLog() {}
29+
disabledLog.__reactDisabledLog = true;
30+
31+
export function disableLogs(): void {
32+
if (disabledDepth === 0) {
33+
/* eslint-disable react-internal/no-production-logging */
34+
prevLog = console.log;
35+
prevInfo = console.info;
36+
prevWarn = console.warn;
37+
prevError = console.error;
38+
prevGroup = console.group;
39+
prevGroupCollapsed = console.groupCollapsed;
40+
prevGroupEnd = console.groupEnd;
41+
// https://github.com/facebook/react/issues/19099
42+
const props = {
43+
configurable: true,
44+
enumerable: true,
45+
value: disabledLog,
46+
writable: true,
47+
};
48+
// $FlowFixMe Flow thinks console is immutable.
49+
Object.defineProperties(console, {
50+
info: props,
51+
log: props,
52+
warn: props,
53+
error: props,
54+
group: props,
55+
groupCollapsed: props,
56+
groupEnd: props,
57+
});
58+
/* eslint-enable react-internal/no-production-logging */
59+
}
60+
disabledDepth++;
61+
}
62+
63+
export function reenableLogs(): void {
64+
disabledDepth--;
65+
if (disabledDepth === 0) {
66+
/* eslint-disable react-internal/no-production-logging */
67+
const props = {
68+
configurable: true,
69+
enumerable: true,
70+
writable: true,
71+
};
72+
// $FlowFixMe Flow thinks console is immutable.
73+
Object.defineProperties(console, {
74+
log: {...props, value: prevLog},
75+
info: {...props, value: prevInfo},
76+
warn: {...props, value: prevWarn},
77+
error: {...props, value: prevError},
78+
group: {...props, value: prevGroup},
79+
groupCollapsed: {...props, value: prevGroupCollapsed},
80+
groupEnd: {...props, value: prevGroupEnd},
81+
});
82+
/* eslint-enable react-internal/no-production-logging */
83+
}
84+
if (disabledDepth < 0) {
85+
console.error(
86+
'disabledDepth fell below zero. ' +
87+
'This is a bug in React. Please file an issue.',
88+
);
89+
}
90+
}

0 commit comments

Comments
 (0)