Print component stacks as error objects to get source mapping

facebook · Jul 5, 2024 · 31c9db8 · 31c9db8
1 parent f38c22b
commit 31c9db8
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 26 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -490,6 +490,7 @@ module.exports = {
         'packages/react-devtools-extensions/**/*.js',
         'packages/react-devtools-shared/src/hook.js',
         'packages/react-devtools-shared/src/backend/console.js',
+        'packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js',
       ],
       globals: {
         __IS_CHROME__: 'readonly',

diff --git a/packages/react-devtools-core/webpack.backend.js b/packages/react-devtools-core/webpack.backend.js
@@ -69,6 +69,8 @@ module.exports = {
       __PROFILE__: false,
       __TEST__: NODE_ENV === 'test',
       __IS_FIREFOX__: false,
+      __IS_CHROME__: false,
+      __IS_EDGE__: false,
       'process.env.DEVTOOLS_PACKAGE': `"react-devtools-core"`,
       'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
       'process.env.GITHUB_URL': `"${GITHUB_URL}"`,

diff --git a/packages/react-devtools-inline/webpack.config.js b/packages/react-devtools-inline/webpack.config.js
@@ -73,6 +73,10 @@ module.exports = {
       __EXTENSION__: false,
       __PROFILE__: false,
       __TEST__: NODE_ENV === 'test',
+      // TODO: Should this be feature tested somehow?
+      __IS_CHROME__: false,
+      __IS_FIREFOX__: false,
+      __IS_EDGE__: false,
       'process.env.DEVTOOLS_PACKAGE': `"react-devtools-inline"`,
       'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
       'process.env.EDITOR_URL': EDITOR_URL != null ? `"${EDITOR_URL}"` : null,

diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js
@@ -1000,7 +1000,7 @@ describe('console', () => {
     );
     expect(mockWarn.mock.calls[1]).toHaveLength(3);
     expect(mockWarn.mock.calls[1][0]).toEqual(
-      '\x1b[2;38;2;124;124;124m%s %s\x1b[0m',
+      '\x1b[2;38;2;124;124;124m%s %o\x1b[0m',
     );
     expect(mockWarn.mock.calls[1][1]).toMatch('warn');
     expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual(
@@ -1014,7 +1014,7 @@ describe('console', () => {
     );
     expect(mockError.mock.calls[1]).toHaveLength(3);
     expect(mockError.mock.calls[1][0]).toEqual(
-      '\x1b[2;38;2;124;124;124m%s %s\x1b[0m',
+      '\x1b[2;38;2;124;124;124m%s %o\x1b[0m',
     );
     expect(mockError.mock.calls[1][1]).toEqual('error');
     expect(normalizeCodeLocInfo(mockError.mock.calls[1][2]).trim()).toEqual(

diff --git a/packages/react-devtools-shared/src/__tests__/utils.js b/packages/react-devtools-shared/src/__tests__/utils.js
@@ -463,6 +463,9 @@ export function overrideFeatureFlags(overrideFlags) {
 }
 
 export function normalizeCodeLocInfo(str) {
+  if (typeof str === 'object' && str !== null) {
+    str = str.stack;
+  }
   if (typeof str !== 'string') {
     return str;
   }

diff --git a/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js b/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js
@@ -29,12 +29,19 @@ export function describeBuiltInComponentFrame(name: string): string {
       prefix = (match && match[1]) || '';
     }
   }
+  let suffix = '';
+  if (__IS_CHROME__ || __IS_EDGE__) {
+    suffix = ' (<anonymous>)';
+  } else if (__IS_FIREFOX__) {
+    suffix = '@unknown:0:0';
+  }
   // We use the prefix to ensure our stacks line up with native stack frames.
-  return '\n' + prefix + name;
+  // We use a suffix to ensure it gets parsed natively.
+  return '\n' + prefix + name + suffix;
 }
 
 export function describeDebugInfoFrame(name: string, env: ?string): string {
-  return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : ''));
+  return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : ''));
 }
 
 let reentry = false;

diff --git a/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js b/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js
@@ -28,6 +28,8 @@ export function describeFiber(
   currentDispatcherRef: CurrentDispatcherRef,
 ): string {
   const {
+    HostHoistable,
+    HostSingleton,
     HostComponent,
     LazyComponent,
     SuspenseComponent,
@@ -40,6 +42,8 @@ export function describeFiber(
   } = workTagMap;
 
   switch (workInProgress.tag) {
+    case HostHoistable:
+    case HostSingleton:
     case HostComponent:
       return describeBuiltInComponentFrame(workInProgress.type);
     case LazyComponent:

diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js
@@ -63,6 +63,15 @@ function isStrictModeOverride(args: Array<any>): boolean {
   }
 }
 
+// We add a suffix to some frames that older versions of React didn't do.
+// To compare if it's equivalent we strip out the suffix to see if they're
+// still equivalent. Similarly, we sometimes use [] and sometimes () so we
+// strip them to for the comparison.
+const frameDiffs = / \(\<anonymous\>\)$|\@unknown\:0\:0$|\(|\)|\[|\]/gm;
+function areStackTracesEqual(a: string, b: string): boolean {
+  return a.replace(frameDiffs, '') === b.replace(frameDiffs, '');
+}
+
 function restorePotentiallyModifiedArgs(args: Array<any>): Array<any> {
   // If the arguments don't have any styles applied, then just copy
   if (!isStrictModeOverride(args)) {
@@ -202,17 +211,11 @@ export function patch({
 
         // $FlowFixMe[missing-local-annot]
         const overrideMethod = (...args) => {
-          let shouldAppendWarningStack = false;
-          if (method !== 'log') {
-            if (consoleSettingsRef.appendComponentStack) {
-              const lastArg = args.length > 0 ? args[args.length - 1] : null;
-              const alreadyHasComponentStack =
-                typeof lastArg === 'string' && isStringComponentStack(lastArg);
-
-              // If we are ever called with a string that already has a component stack,
-              // e.g. a React error/warning, don't append a second stack.
-              shouldAppendWarningStack = !alreadyHasComponentStack;
-            }
+          let alreadyHasComponentStack = false;
+          if (method !== 'log' && consoleSettingsRef.appendComponentStack) {
+            const lastArg = args.length > 0 ? args[args.length - 1] : null;
+            alreadyHasComponentStack =
+              typeof lastArg === 'string' && isStringComponentStack(lastArg); // The last argument should be a component stack.
           }
 
           const shouldShowInlineWarningsAndErrors =
@@ -242,7 +245,7 @@ export function patch({
                 }
 
                 if (
-                  shouldAppendWarningStack &&
+                  consoleSettingsRef.appendComponentStack &&
                   !supportsNativeConsoleTasks(current)
                 ) {
                   const componentStack = getStackByFiberInDevAndProd(
@@ -251,17 +254,60 @@ export function patch({
                     (currentDispatcherRef: any),
                   );
                   if (componentStack !== '') {
-                    if (isStrictModeOverride(args)) {
-                      if (__IS_FIREFOX__) {
-                        args[0] = `${args[0]} %s`;
-                        args.push(componentStack);
-                      } else {
-                        args[0] =
-                          ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK;
-                        args.push(componentStack);
+                    // Create a fake Error so that when we print it we get native source maps. Every
+                    // browser will print the .stack property of the error and then parse it back for source
+                    // mapping. Rather than print the internal slot. So it doesn't matter that the internal
+                    // slot doesn't line up.
+                    const fakeError = new Error('');
+                    // In Chromium, only the stack property is printed but in Firefox the <name>:<message>
+                    // gets printed so to make the colon make sense, we name it so we print Component Stack:
+                    // and similarly Safari leave an expandable slot.
+                    fakeError.name = 'Component Stack'; // This gets printed
+                    // In Chromium, the stack property needs to start with ^[\w.]*Error\b to trigger stack
+                    // formatting. Otherwise it is left alone. So we prefix it. Otherwise we just override it
+                    // to our own stack.
+                    fakeError.stack =
+                      __IS_CHROME__ || __IS_EDGE__
+                        ? 'Error Component Stack:' + componentStack
+                        : componentStack;
+                    if (alreadyHasComponentStack) {
+                      // Only modify the component stack if it matches what we would've added anyway.
+                      // Otherwise we assume it was a non-React stack.
+                      if (
+                        areStackTracesEqual(
+                          args[args.length - 1],
+                          componentStack,
+                        )
+                      ) {
+                        args[args.length - 1] = fakeError;
+                        if (isStrictModeOverride(args)) {
+                          if (__IS_FIREFOX__) {
+                            args[0] = `${args[0]} %o`;
+                          } else {
+                            args[0] =
+                              ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK;
+                          }
+                        } else {
+                          const firstArg = args[0];
+                          if (
+                            args.length > 1 &&
+                            typeof firstArg === 'string' &&
+                            firstArg.endsWith('%s')
+                          ) {
+                            args[0] = firstArg.slice(0, firstArg.length - 2); // Strip the %s param
+                          }
+                        }
                       }
                     } else {
-                      args.push(componentStack);
+                      args.push(fakeError);
+                      if (isStrictModeOverride(args)) {
+                        if (__IS_FIREFOX__) {
+                          args[0] = `${args[0]} %o`;
+                        } else {
+                          args[0] =
+                            ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK;
+                        }
+                      }
                     }
                   }
                 }

diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js
@@ -62,4 +62,4 @@ export const PROFILER_EXPORT_VERSION = 5;
 export const FIREFOX_CONSOLE_DIMMING_COLOR = 'color: rgba(124, 124, 124, 0.75)';
 export const ANSI_STYLE_DIMMING_TEMPLATE = '\x1b[2;38;2;124;124;124m%s\x1b[0m';
 export const ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK =
-  '\x1b[2;38;2;124;124;124m%s %s\x1b[0m';
+  '\x1b[2;38;2;124;124;124m%s %o\x1b[0m';
diff --git a/scripts/flow/react-devtools.js b/scripts/flow/react-devtools.js
@@ -13,3 +13,5 @@ declare const __EXTENSION__: boolean;
 declare const __TEST__: boolean;
 
 declare const __IS_FIREFOX__: boolean;
+declare const __IS_CHROME__: boolean;
+declare const __IS_EDGE__: boolean;
diff --git a/scripts/jest/devtools/setupEnv.js b/scripts/jest/devtools/setupEnv.js
@@ -12,6 +12,8 @@ if (!global.hasOwnProperty('localStorage')) {
 global.__DEV__ = process.env.NODE_ENV !== 'production';
 global.__TEST__ = true;
 global.__IS_FIREFOX__ = false;
+global.__IS_CHROME__ = false;
+global.__IS_EDGE__ = false;
 
 const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion;