@samitouri / QOS-React-2 / commits / 491a4eacce

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

Sebastian Markbåge committed Jul 8, 2024 at 18:42 UTC 491a4eacce69fec4144725beaac7141da269e8cf
11 files changed +92 -26
.eslintrc.js
+1
@@ -490,6 +490,7 @@ module.exports = {
490 'packages/react-devtools-extensions/**/*.js',
491 'packages/react-devtools-shared/src/hook.js',
492 'packages/react-devtools-shared/src/backend/console.js',
493 + 'packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js',
494 ],
495 globals: {
496 __IS_CHROME__: 'readonly',
packages/react-devtools-core/webpack.backend.js
+2
@@ -69,6 +69,8 @@ module.exports = {
69 __PROFILE__: false,
70 __TEST__: NODE_ENV === 'test',
71 __IS_FIREFOX__: false,
72 + __IS_CHROME__: false,
73 + __IS_EDGE__: false,
74 'process.env.DEVTOOLS_PACKAGE': `"react-devtools-core"`,
75 'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
76 'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
packages/react-devtools-inline/webpack.config.js
+4
@@ -73,6 +73,10 @@ module.exports = {
73 __EXTENSION__: false,
74 __PROFILE__: false,
75 __TEST__: NODE_ENV === 'test',
76 + // TODO: Should this be feature tested somehow?
77 + __IS_CHROME__: false,
78 + __IS_FIREFOX__: false,
79 + __IS_EDGE__: false,
80 'process.env.DEVTOOLS_PACKAGE': `"react-devtools-inline"`,
81 'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
82 'process.env.EDITOR_URL': EDITOR_URL != null ? `"${EDITOR_URL}"` : null,
packages/react-devtools-shared/src/__tests__/console-test.js
+2 -2
@@ -1000,7 +1000,7 @@ describe('console', () => {
1000 );
1001 expect(mockWarn.mock.calls[1]).toHaveLength(3);
1002 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',
1004 );
1005 expect(mockWarn.mock.calls[1][1]).toMatch('warn');
1006 expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual(
@@ -1014,7 +1014,7 @@ describe('console', () => {
1014 );
1015 expect(mockError.mock.calls[1]).toHaveLength(3);
1016 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',
1018 );
1019 expect(mockError.mock.calls[1][1]).toEqual('error');
1020 expect(normalizeCodeLocInfo(mockError.mock.calls[1][2]).trim()).toEqual(
packages/react-devtools-shared/src/__tests__/utils.js
+3
@@ -463,6 +463,9 @@ export function overrideFeatureFlags(overrideFlags) {
463 }
464
465 export function normalizeCodeLocInfo(str) {
466 + if (typeof str === 'object' && str !== null) {
467 + str = str.stack;
468 + }
469 if (typeof str !== 'string') {
470 return str;
471 }
packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js
+9 -2
@@ -29,12 +29,19 @@ export function describeBuiltInComponentFrame(name: string): string {
29 prefix = (match && match[1]) || '';
30 }
31 }
32 + let suffix = '';
33 + if (__IS_CHROME__ || __IS_EDGE__) {
34 + suffix = ' (<anonymous>)';
35 + } else if (__IS_FIREFOX__) {
36 + suffix = '@unknown:0:0';
37 + }
38 // 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;
41 }
42
43 export function describeDebugInfoFrame(name: string, env: ?string): string {
37 - return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : ''));
44 + return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : ''));
45 }
46
47 let reentry = false;
packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js
+4
@@ -28,6 +28,8 @@ export function describeFiber(
28 currentDispatcherRef: CurrentDispatcherRef,
29 ): string {
30 const {
31 + HostHoistable,
32 + HostSingleton,
33 HostComponent,
34 LazyComponent,
35 SuspenseComponent,
@@ -40,6 +42,8 @@ export function describeFiber(
42 } = workTagMap;
43
44 switch (workInProgress.tag) {
45 + case HostHoistable:
46 + case HostSingleton:
47 case HostComponent:
48 return describeBuiltInComponentFrame(workInProgress.type);
49 case LazyComponent:
packages/react-devtools-shared/src/backend/console.js
+62 -21
@@ -63,6 +63,15 @@ function isStrictModeOverride(args: Array<any>): boolean {
63 }
64 }
65
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 +
75 function restorePotentiallyModifiedArgs(args: Array<any>): Array<any> {
76 // If the arguments don't have any styles applied, then just copy
77 if (!isStrictModeOverride(args)) {
@@ -204,17 +213,11 @@ export function patch({
213
214 // $FlowFixMe[missing-local-annot]
215 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.
221 }
222
223 const shouldShowInlineWarningsAndErrors =
@@ -244,7 +247,7 @@ export function patch({
247 }
248
249 if (
247 - shouldAppendWarningStack &&
250 + consoleSettingsRef.appendComponentStack &&
251 !supportsNativeConsoleTasks(current)
252 ) {
253 const componentStack = getStackByFiberInDevAndProd(
@@ -253,17 +256,55 @@ export function patch({
256 (currentDispatcherRef: any),
257 );
258 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;
297 }
298 } 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 + }
308 }
309 }
310 }
packages/react-devtools-shared/src/constants.js
+1 -1
@@ -62,4 +62,4 @@ export const PROFILER_EXPORT_VERSION = 5;
62 export const FIREFOX_CONSOLE_DIMMING_COLOR = 'color: rgba(124, 124, 124, 0.75)';
63 export const ANSI_STYLE_DIMMING_TEMPLATE = '\x1b[2;38;2;124;124;124m%s\x1b[0m';
64 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
+2
@@ -13,3 +13,5 @@ declare const __EXTENSION__: boolean;
13 declare const __TEST__: boolean;
14
15 declare const __IS_FIREFOX__: boolean;
16 +declare const __IS_CHROME__: boolean;
17 +declare const __IS_EDGE__: boolean;
scripts/jest/devtools/setupEnv.js
+2
@@ -12,6 +12,8 @@ if (!global.hasOwnProperty('localStorage')) {
12 global.__DEV__ = process.env.NODE_ENV !== 'production';
13 global.__TEST__ = true;
14 global.__IS_FIREFOX__ = false;
15 +global.__IS_CHROME__ = false;
16 +global.__IS_EDGE__ = false;
17
18 const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion;
19