Skip to content

Commit 491a4ea

Browse files
sebmarkbagesebmarkbage
authored
[DevTools] Print component stacks as error objects to get source mapping (#30289)
Before: <img width="844" alt="Screenshot 2024-07-04 at 3 20 34 PM" src="https://github.com/facebook/react/assets/63648/0fd8a53f-538a-4429-a4cf-c22f85a09aa8"> After: <img width="845" alt="Screenshot 2024-07-05 at 6 08 28 PM" src="https://github.com/facebook/react/assets/63648/7b9da13a-fa97-4581-9899-06de6fface65"> Firefox: <img width="1338" alt="Screenshot 2024-07-05 at 6 09 50 PM" src="https://github.com/facebook/react/assets/63648/f2eb9f2a-2251-408f-86d0-b081279ba378"> The first log doesn't get a stack because it's logged before DevTools boots up and connects which is unfortunate. The second log already has a stack printed by React (this is on stable) it gets replaced by our object now. The third and following logs don't have a stack and get one appended. I only turn the stack into an error object if it matches what we would emit from DevTools anyway. Otherwise we assume it's not React. Since I had to change the format slightly to make this work, I first normalize the stack slightly before doing a comparison since it won't be 1:1.
1 parent 274c980 commit 491a4ea

File tree

11 files changed

+92
-26
lines changed

11 files changed

+92
-26
lines changed

‎.eslintrc.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ module.exports = {
490490
'packages/react-devtools-extensions/**/*.js',
491491
'packages/react-devtools-shared/src/hook.js',
492492
'packages/react-devtools-shared/src/backend/console.js',
493+
'packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js',
493494
],
494495
globals: {
495496
__IS_CHROME__: 'readonly',

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ module.exports = {
6969
__PROFILE__: false,
7070
__TEST__: NODE_ENV === 'test',
7171
__IS_FIREFOX__: false,
72+
__IS_CHROME__: false,
73+
__IS_EDGE__: false,
7274
'process.env.DEVTOOLS_PACKAGE': `"react-devtools-core"`,
7375
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
7476
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,

‎packages/react-devtools-inline/webpack.config.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ module.exports = {
7373
__EXTENSION__: false,
7474
__PROFILE__: false,
7575
__TEST__: NODE_ENV === 'test',
76+
// TODO: Should this be feature tested somehow?
77+
__IS_CHROME__: false,
78+
__IS_FIREFOX__: false,
79+
__IS_EDGE__: false,
7680
'process.env.DEVTOOLS_PACKAGE': `"react-devtools-inline"`,
7781
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
7882
'process.env.EDITOR_URL': EDITOR_URL != null ? `"${EDITOR_URL}"` : null,

‎packages/react-devtools-shared/src/__tests__/console-test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ describe('console', () => {
10001000
);
10011001
expect(mockWarn.mock.calls[1]).toHaveLength(3);
10021002
expect(mockWarn.mock.calls[1][0]).toEqual(
1003-
'\x1b[2;38;2;124;124;124m%s %s\x1b[0m',
1003+
'\x1b[2;38;2;124;124;124m%s %o\x1b[0m',
10041004
);
10051005
expect(mockWarn.mock.calls[1][1]).toMatch('warn');
10061006
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual(
@@ -1014,7 +1014,7 @@ describe('console', () => {
10141014
);
10151015
expect(mockError.mock.calls[1]).toHaveLength(3);
10161016
expect(mockError.mock.calls[1][0]).toEqual(
1017-
'\x1b[2;38;2;124;124;124m%s %s\x1b[0m',
1017+
'\x1b[2;38;2;124;124;124m%s %o\x1b[0m',
10181018
);
10191019
expect(mockError.mock.calls[1][1]).toEqual('error');
10201020
expect(normalizeCodeLocInfo(mockError.mock.calls[1][2]).trim()).toEqual(

‎packages/react-devtools-shared/src/__tests__/utils.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,9 @@ export function overrideFeatureFlags(overrideFlags) {
463463
}
464464

465465
export function normalizeCodeLocInfo(str) {
466+
if (typeof str === 'object' && str !== null) {
467+
str = str.stack;
468+
}
466469
if (typeof str !== 'string') {
467470
return str;
468471
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,19 @@ export function describeBuiltInComponentFrame(name: string): string {
2929
prefix = (match && match[1]) || '';
3030
}
3131
}
32+
let suffix = '';
33+
if (__IS_CHROME__ || __IS_EDGE__) {
34+
suffix = ' (<anonymous>)';
35+
} else if (__IS_FIREFOX__) {
36+
suffix = '@unknown:0:0';
37+
}
3238
// We use the prefix to ensure our stacks line up with native stack frames.
33-
return '\n' + prefix + name;
39+
// We use a suffix to ensure it gets parsed natively.
40+
return '\n' + prefix + name + suffix;
3441
}
3542

3643
export function describeDebugInfoFrame(name: string, env: ?string): string {
37-
return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : ''));
44+
return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : ''));
3845
}
3946

4047
let reentry = false;

‎packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export function describeFiber(
2828
currentDispatcherRef: CurrentDispatcherRef,
2929
): string {
3030
const {
31+
HostHoistable,
32+
HostSingleton,
3133
HostComponent,
3234
LazyComponent,
3335
SuspenseComponent,
@@ -40,6 +42,8 @@ export function describeFiber(
4042
} = workTagMap;
4143

4244
switch (workInProgress.tag) {
45+
case HostHoistable:
46+
case HostSingleton:
4347
case HostComponent:
4448
return describeBuiltInComponentFrame(workInProgress.type);
4549
case LazyComponent:

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

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ function isStrictModeOverride(args: Array<any>): boolean {
6363
}
6464
}
6565

66+
// We add a suffix to some frames that older versions of React didn't do.
67+
// To compare if it's equivalent we strip out the suffix to see if they're
68+
// still equivalent. Similarly, we sometimes use [] and sometimes () so we
69+
// strip them to for the comparison.
70+
const frameDiffs = / \(\<anonymous\>\)$|\@unknown\:0\:0$|\(|\)|\[|\]/gm;
71+
function areStackTracesEqual(a: string, b: string): boolean {
72+
return a.replace(frameDiffs, '') === b.replace(frameDiffs, '');
73+
}
74+
6675
function restorePotentiallyModifiedArgs(args: Array<any>): Array<any> {
6776
// If the arguments don't have any styles applied, then just copy
6877
if (!isStrictModeOverride(args)) {
@@ -204,17 +213,11 @@ export function patch({
204213

205214
// $FlowFixMe[missing-local-annot]
206215
const overrideMethod = (...args) => {
207-
let shouldAppendWarningStack = false;
208-
if (method !== 'log') {
209-
if (consoleSettingsRef.appendComponentStack) {
210-
const lastArg = args.length > 0 ? args[args.length - 1] : null;
211-
const alreadyHasComponentStack =
212-
typeof lastArg === 'string' && isStringComponentStack(lastArg);
213-
214-
// If we are ever called with a string that already has a component stack,
215-
// e.g. a React error/warning, don't append a second stack.
216-
shouldAppendWarningStack = !alreadyHasComponentStack;
217-
}
216+
let alreadyHasComponentStack = false;
217+
if (method !== 'log' && consoleSettingsRef.appendComponentStack) {
218+
const lastArg = args.length > 0 ? args[args.length - 1] : null;
219+
alreadyHasComponentStack =
220+
typeof lastArg === 'string' && isStringComponentStack(lastArg); // The last argument should be a component stack.
218221
}
219222

220223
const shouldShowInlineWarningsAndErrors =
@@ -244,7 +247,7 @@ export function patch({
244247
}
245248

246249
if (
247-
shouldAppendWarningStack &&
250+
consoleSettingsRef.appendComponentStack &&
248251
!supportsNativeConsoleTasks(current)
249252
) {
250253
const componentStack = getStackByFiberInDevAndProd(
@@ -253,17 +256,55 @@ export function patch({
253256
(currentDispatcherRef: any),
254257
);
255258
if (componentStack !== '') {
256-
if (isStrictModeOverride(args)) {
257-
if (__IS_FIREFOX__) {
258-
args[0] = `${args[0]} %s`;
259-
args.push(componentStack);
260-
} else {
261-
args[0] =
262-
ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK;
263-
args.push(componentStack);
259+
// Create a fake Error so that when we print it we get native source maps. Every
260+
// browser will print the .stack property of the error and then parse it back for source
261+
// mapping. Rather than print the internal slot. So it doesn't matter that the internal
262+
// slot doesn't line up.
263+
const fakeError = new Error('');
264+
// In Chromium, only the stack property is printed but in Firefox the <name>:<message>
265+
// gets printed so to make the colon make sense, we name it so we print Component Stack:
266+
// and similarly Safari leave an expandable slot.
267+
fakeError.name = 'Component Stack'; // This gets printed
268+
// In Chromium, the stack property needs to start with ^[\w.]*Error\b to trigger stack
269+
// formatting. Otherwise it is left alone. So we prefix it. Otherwise we just override it
270+
// to our own stack.
271+
fakeError.stack =
272+
__IS_CHROME__ || __IS_EDGE__
273+
? 'Error Component Stack:' + componentStack
274+
: componentStack;
275+
if (alreadyHasComponentStack) {
276+
// Only modify the component stack if it matches what we would've added anyway.
277+
// Otherwise we assume it was a non-React stack.
278+
if (isStrictModeOverride(args)) {
279+
// We do nothing to Strict Mode overrides that already has a stack
280+
// because we have already lost some context for how to format it
281+
// since we've already merged the stack into the log at this point.
282+
} else if (
283+
areStackTracesEqual(
284+
args[args.length - 1],
285+
componentStack,
286+
)
287+
) {
288+
const firstArg = args[0];
289+
if (
290+
args.length > 1 &&
291+
typeof firstArg === 'string' &&
292+
firstArg.endsWith('%s')
293+
) {
294+
args[0] = firstArg.slice(0, firstArg.length - 2); // Strip the %s param
295+
}
296+
args[args.length - 1] = fakeError;
264297
}
265298
} else {
266-
args.push(componentStack);
299+
args.push(fakeError);
300+
if (isStrictModeOverride(args)) {
301+
if (__IS_FIREFOX__) {
302+
args[0] = `${args[0]} %o`;
303+
} else {
304+
args[0] =
305+
ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK;
306+
}
307+
}
267308
}
268309
}
269310
}

‎packages/react-devtools-shared/src/constants.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@ export const PROFILER_EXPORT_VERSION = 5;
6262
export const FIREFOX_CONSOLE_DIMMING_COLOR = 'color: rgba(124, 124, 124, 0.75)';
6363
export const ANSI_STYLE_DIMMING_TEMPLATE = '\x1b[2;38;2;124;124;124m%s\x1b[0m';
6464
export const ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK =
65-
'\x1b[2;38;2;124;124;124m%s %s\x1b[0m';
65+
'\x1b[2;38;2;124;124;124m%s %o\x1b[0m';

‎scripts/flow/react-devtools.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,5 @@ declare const __EXTENSION__: boolean;
1313
declare const __TEST__: boolean;
1414

1515
declare const __IS_FIREFOX__: boolean;
16+
declare const __IS_CHROME__: boolean;
17+
declare const __IS_EDGE__: boolean;

0 commit comments

Comments
 (0)