Move getComponentStack computation into renderer

The console patching shouldn't need to know about Fibers.

Author: sebmarkbage

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

@@ -54,14 +54,6 @@ describe('console', () => {
       fakeConsole,
     );
 
-    const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject;
-    global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => {
-      rendererID = inject(internals);
-
-      Console.registerRenderer(internals);
-      return rendererID;
-    };
-
     React = require('react');
     if (
       React.version.startsWith('19') &&
@@ -1100,9 +1092,18 @@ describe('console error', () => {
     global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => {
       inject(internals);
 
-      Console.registerRenderer(internals, () => {
-        throw Error('foo');
-      });
+      Console.registerRenderer(
+        internals,
+        () => {
+          throw Error('foo');
+        },
+        () => {
+          return {
+            enableOwnerStacks: true,
+            componentStack: '\n    at FakeStack (fake-file)',
+          };
+        },
+      );
     };
 
     React = require('react');
@@ -1146,14 +1147,14 @@ describe('console error', () => {
     expect(mockWarn.mock.calls[0][0]).toBe('warn');
     // An error in showInlineWarningsAndErrors doesn't need to break component stacks.
     expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
-      '\n    in App (at **)',
+      '\n    in FakeStack (at **)',
     );
 
     expect(mockError).toHaveBeenCalledTimes(1);
     expect(mockError.mock.calls[0]).toHaveLength(2);
     expect(mockError.mock.calls[0][0]).toBe('error');
     expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
-      '\n    in App (at **)',
+      '\n    in FakeStack (at **)',
     );
   });
 });

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

@@ -25,14 +25,7 @@ import {
   ANSI_STYLE_DIMMING_TEMPLATE,
   ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK,
 } from 'react-devtools-shared/src/constants';
-import {getInternalReactConstants, getDispatcherRef} from './fiber/renderer';
-import {
-  getStackByFiberInDevAndProd,
-  getOwnerStackByFiberInDev,
-  supportsOwnerStacks,
-  supportsConsoleTasks,
-} from './fiber/DevToolsFiberComponentStack';
-import {formatOwnerStack} from './shared/DevToolsOwnerStack';
+import {getInternalReactConstants} from './fiber/renderer';
 import {castBool, castBrowserTheme} from '../utils';
 
 const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn'];
@@ -91,6 +84,9 @@ function restorePotentiallyModifiedArgs(args: Array<any>): Array<any> {
 }
 
 type OnErrorOrWarning = (type: 'error' | 'warn', args: Array<any>) => void;
+type GetComponentStack = (
+  topFrame: Error,
+) => null | {enableOwnerStacks: boolean, componentStack: string};
 
 const injectedRenderers: Map<
   ReactRenderer,
@@ -99,6 +95,7 @@ const injectedRenderers: Map<
     getCurrentFiber: () => Fiber | null,
     onErrorOrWarning: ?OnErrorOrWarning,
     workTagMap: WorkTagMap,
+    getComponentStack: ?GetComponentStack,
   },
 > = new Map();
 
@@ -130,6 +127,7 @@ export function dangerous_setTargetConsoleForTesting(
 export function registerRenderer(
   renderer: ReactRenderer,
   onErrorOrWarning?: OnErrorOrWarning,
+  getComponentStack?: GetComponentStack,
 ): void {
   const {currentDispatcherRef, getCurrentFiber, version} = renderer;
 
@@ -143,6 +141,7 @@ export function registerRenderer(
       getCurrentFiber,
       workTagMap: ReactTypeOfWork,
       onErrorOrWarning,
+      getComponentStack,
     });
   }
 }
@@ -217,13 +216,12 @@ export function patch({
           // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
           // eslint-disable-next-line no-for-of-loops/no-for-of-loops
           for (const renderer of injectedRenderers.values()) {
-            const currentDispatcherRef = getDispatcherRef(renderer);
-            const {getCurrentFiber, onErrorOrWarning, workTagMap} = renderer;
+            const {getComponentStack, onErrorOrWarning} = renderer;
             try {
               if (shouldShowInlineWarningsAndErrors) {
                 // patch() is called by two places: (1) the hook and (2) the renderer backend.
                 // The backend is what implements a message queue, so it's the only one that injects onErrorOrWarning.
-                if (typeof onErrorOrWarning === 'function') {
+                if (onErrorOrWarning != null) {
                   onErrorOrWarning(
                     ((method: any): 'error' | 'warn'),
                     // Restore and copy args before we mutate them (e.g. adding the component stack)
@@ -237,46 +235,26 @@ export function patch({
                 throw error;
               }, 0);
             }
-            const current: ?Fiber = getCurrentFiber();
-            if (current != null) {
-              try {
-                if (
-                  consoleSettingsRef.appendComponentStack &&
-                  !supportsConsoleTasks(current)
-                ) {
-                  const enableOwnerStacks = supportsOwnerStacks(current);
-                  let componentStack = '';
-                  if (enableOwnerStacks) {
-                    // Prefix the owner stack with the current stack. I.e. what called
-                    // console.error. While this will also be part of the native stack,
-                    // it is hidden and not presented alongside this argument so we print
-                    // them all together.
-                    const topStackFrames = formatOwnerStack(
-                      new Error('react-stack-top-frame'),
-                    );
-                    if (topStackFrames) {
-                      componentStack += '\n' + topStackFrames;
-                    }
-                    componentStack += getOwnerStackByFiberInDev(
-                      workTagMap,
-                      current,
-                      (currentDispatcherRef: any),
-                    );
-                  } else {
-                    componentStack = getStackByFiberInDevAndProd(
-                      workTagMap,
-                      current,
-                      (currentDispatcherRef: any),
-                    );
-                  }
+            try {
+              if (
+                consoleSettingsRef.appendComponentStack &&
+                getComponentStack != null
+              ) {
+                // This needs to be directly in the wrapper so we can pop exactly one frame.
+                const topFrame = Error('react-stack-top-frame');
+                const match = getComponentStack(topFrame);
+                if (match !== null) {
+                  const {enableOwnerStacks, componentStack} = match;
+                  // Empty string means we have a match but no component stack.
+                  // We don't need to look in other renderers but we also don't add anything.
                   if (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:
+                    // gets printed so to make the colon make sense, we name it so we print Stack:
                     // and similarly Safari leave an expandable slot.
                     fakeError.name = enableOwnerStacks
                       ? 'Stack'
@@ -290,6 +268,7 @@ export function patch({
                             ? 'Error Stack:'
                             : '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.
@@ -325,15 +304,15 @@ export function patch({
                       }
                     }
                   }
+                  // Don't add stacks from other renderers.
+                  break;
                 }
-              } catch (error) {
-                // Don't let a DevTools or React internal error interfere with logging.
-                setTimeout(() => {
-                  throw error;
-                }, 0);
-              } finally {
-                break;
               }
+            } catch (error) {
+              // Don't let a DevTools or React internal error interfere with logging.
+              setTimeout(() => {
+                throw error;
+              }, 0);
             }
           }
 

packages/react-devtools-shared/src/backend/fiber/renderer.js

@@ -103,6 +103,13 @@ import {enableStyleXFeatures} from 'react-devtools-feature-flags';
 import is from 'shared/objectIs';
 import hasOwnProperty from 'shared/hasOwnProperty';
 
+import {
+  getStackByFiberInDevAndProd,
+  getOwnerStackByFiberInDev,
+  supportsOwnerStacks,
+  supportsConsoleTasks,
+} from './DevToolsFiberComponentStack';
+
 // $FlowFixMe[method-unbinding]
 const toString = Object.prototype.toString;
 
@@ -1063,6 +1070,56 @@ export function attach(
     }
   }
 
+  function getComponentStack(
+    topFrame: Error,
+  ): null | {enableOwnerStacks: boolean, componentStack: string} {
+    if (getCurrentFiber === undefined) {
+      // Expected this to be part of the renderer. Ignore.
+      return null;
+    }
+    const current = getCurrentFiber();
+    if (current === null) {
+      // Outside of our render scope.
+      return null;
+    }
+
+    if (supportsConsoleTasks(current)) {
+      // This will be handled natively by console.createTask. No need for
+      // DevTools to add it.
+      return null;
+    }
+
+    const dispatcherRef = getDispatcherRef(renderer);
+    if (dispatcherRef === undefined) {
+      return null;
+    }
+
+    const enableOwnerStacks = supportsOwnerStacks(current);
+    let componentStack = '';
+    if (enableOwnerStacks) {
+      // Prefix the owner stack with the current stack. I.e. what called
+      // console.error. While this will also be part of the native stack,
+      // it is hidden and not presented alongside this argument so we print
+      // them all together.
+      const topStackFrames = formatOwnerStack(topFrame);
+      if (topStackFrames) {
+        componentStack += '\n' + topStackFrames;
+      }
+      componentStack += getOwnerStackByFiberInDev(
+        ReactTypeOfWork,
+        current,
+        dispatcherRef,
+      );
+    } else {
+      componentStack = getStackByFiberInDevAndProd(
+        ReactTypeOfWork,
+        current,
+        dispatcherRef,
+      );
+    }
+    return {enableOwnerStacks, componentStack};
+  }
+
   // Called when an error or warning is logged during render, commit, or passive (including unmount functions).
   function onErrorOrWarning(
     type: 'error' | 'warn',
@@ -1128,7 +1185,7 @@ export function attach(
   // Patching the console enables DevTools to do a few useful things:
   // * Append component stacks to warnings and error messages
   // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber)
-  registerRendererWithConsole(renderer, onErrorOrWarning);
+  registerRendererWithConsole(renderer, onErrorOrWarning, getComponentStack);
 
   // The renderer interface can't read these preferences directly,
   // because it is stored in localStorage within the context of the extension.