Remove Component Stack from React Logged Warnings and Error Reporting

packages/internal-test-utils/consoleMock.js

@@ -44,6 +44,34 @@ const patchConsoleMethod = (
       return;
     }
 
+    // Append Component Stacks. Simulates a framework or DevTools appending them.
+    if (
+      typeof format === 'string' &&
+      (methodName === 'error' || methodName === 'warn')
+    ) {
+      const React = require('react');
+      if (React.captureOwnerStack) {
+        // enableOwnerStacks enabled. When it's always on, we can assume this case.
+        const stack = React.captureOwnerStack();
+        if (stack) {
+          format += '%s';
+          args.push(stack);
+        }
+      } else {
+        // Otherwise we have to use internals to emulate parent stacks.
+        const ReactSharedInternals =
+          React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE ||
+          React.__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
+        if (ReactSharedInternals && ReactSharedInternals.getCurrentStack) {
+          const stack = ReactSharedInternals.getCurrentStack();
+          if (stack !== '') {
+            format += '%s';
+            args.push(stack);
+          }
+        }
+      }
+    }
+
     // Capture the call stack now so we can warn about it later.
     // The call stack has helpful information for the test author.
     // Don't throw yet though b'c it might be accidentally caught and suppressed.
@@ -204,7 +232,7 @@ export function assertConsoleLogsCleared() {
     if (warnings.length > 0) {
       message += `\nconsole.warn was called without assertConsoleWarnDev:\n${diff(
         '',
-        warnings.join('\n'),
+        warnings.map(normalizeComponentStack).join('\n'),
         {
           omitAnnotationLines: true,
         },
@@ -213,7 +241,7 @@ export function assertConsoleLogsCleared() {
     if (errors.length > 0) {
       message += `\nconsole.error was called without assertConsoleErrorDev:\n${diff(
         '',
-        errors.join('\n'),
+        errors.map(normalizeComponentStack).join('\n'),
         {
           omitAnnotationLines: true,
         },
@@ -249,6 +277,19 @@ function normalizeCodeLocInfo(str) {
   });
 }
 
+function normalizeComponentStack(entry) {
+  if (
+    typeof entry[0] === 'string' &&
+    entry[0].endsWith('%s') &&
+    isLikelyAComponentStack(entry[entry.length - 1])
+  ) {
+    const clone = entry.slice(0);
+    clone[clone.length - 1] = normalizeCodeLocInfo(entry[entry.length - 1]);
+    return clone;
+  }
+  return entry;
+}
+
 const isLikelyAComponentStack = message =>
   typeof message === 'string' &&
   (message.indexOf('<component stack>') > -1 ||

packages/react-client/src/__tests__/ReactFlight-test.js

@@ -1436,12 +1436,23 @@ describe('ReactFlight', () => {
 
   it('should warn in DEV a child is missing keys on server component', () => {
     function NoKey({children}) {
-      return <div key="this has a key but parent doesn't" />;
+      return ReactServer.createElement('div', {
+        key: "this has a key but parent doesn't",
+      });
     }
     expect(() => {
+      // While we're on the server we need to have the Server version active to track component stacks.
+      jest.resetModules();
+      jest.mock('react', () => ReactServer);
       const transport = ReactNoopFlightServer.render(
-        <div>{Array(6).fill(<NoKey />)}</div>,
+        ReactServer.createElement(
+          'div',
+          null,
+          Array(6).fill(ReactServer.createElement(NoKey)),
+        ),
       );
+      jest.resetModules();
+      jest.mock('react', () => React);
       ReactNoopFlightClient.read(transport);
     }).toErrorDev('Each child in a list should have a unique "key" prop.');
   });
@@ -2814,7 +2825,7 @@ describe('ReactFlight', () => {
   });
 
   // @gate (enableOwnerStacks && enableServerComponentLogs) || !__DEV__
-  it('should not include component stacks in replayed logs (unless DevTools add them)', () => {
+  it('should include only one component stack in replayed logs (if DevTools or polyfill adds them)', () => {
     class MyError extends Error {
       toJSON() {
         return 123;
@@ -2839,6 +2850,9 @@ describe('ReactFlight', () => {
       return ReactServer.createElement(Bar);
     }
 
+    // While we're on the server we need to have the Server version active to track component stacks.
+    jest.resetModules();
+    jest.mock('react', () => ReactServer);
     const transport = ReactNoopFlightServer.render(
       ReactServer.createElement(App),
     );
@@ -2857,6 +2871,8 @@ describe('ReactFlight', () => {
     ]);
 
     // Replay logs on the client
+    jest.resetModules();
+    jest.mock('react', () => React);
     ReactNoopFlightClient.read(transport);
     assertConsoleErrorDev(
       [
@@ -2866,8 +2882,8 @@ describe('ReactFlight', () => {
           '  <div>Womp womp: {Error}</div>\n' +
           '                  ^^^^^^^',
       ],
-      // We should not have a stack in the replay because that should be added either by console.createTask
-      // or React DevTools on the client. Neither of which we do here.
+      // We should have a stack in the replay but we don't yet set the owner from the Flight replaying
+      // so our simulated polyfill doesn't end up getting any component stacks yet.
       {withoutStack: true},
     );
   });

packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js

@@ -143,7 +143,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             expect.stringContaining('%s'),
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -208,7 +209,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -274,7 +276,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             expect.stringContaining('%s'),
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -344,7 +347,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -410,7 +414,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             expect.stringContaining('%s'),
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -478,7 +483,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {

packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js

@@ -162,7 +162,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             // Addendum by React:
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
 
@@ -239,7 +240,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -309,7 +311,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             // Addendum by React:
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
 
@@ -390,7 +393,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {
@@ -460,7 +464,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
             // Addendum by React:
             expect.stringContaining('An error occurred in the <Foo> component'),
             expect.stringContaining('Consider adding an error boundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
 
@@ -540,7 +545,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
               'The above error occurred in the <Foo> component',
             ),
             expect.stringContaining('ErrorBoundary'),
-            expect.stringContaining('Foo'),
+            // The component stack is not added without the polyfill/devtools.
+            // expect.stringContaining('Foo'),
           ],
         ]);
       } else {

packages/react-dom/src/__tests__/ReactUpdates-test.js

@@ -1857,12 +1857,14 @@ describe('ReactUpdates', () => {
       }
 
       let error = null;
-      let stack = null;
+      let ownerStack = null;
       let nativeStack = null;
       const originalConsoleError = console.error;
-      console.error = (e, s) => {
+      console.error = e => {
         error = e;
-        stack = s;
+        ownerStack = gate(flags => flags.enableOwnerStacks)
+          ? React.captureOwnerStack()
+          : null;
         nativeStack = new Error().stack;
         Scheduler.log('stop');
       };
@@ -1878,12 +1880,11 @@ describe('ReactUpdates', () => {
       expect(error).toContain('Maximum update depth exceeded');
       // The currently executing effect should be on the native stack
       expect(nativeStack).toContain('at myEffect');
-      if (!gate(flags => flags.enableOwnerStacks)) {
-        // The currently running component's name is not in the owner
-        // stack because it's just its JSX callsite.
-        expect(stack).toContain('at NonTerminating');
+      if (gate(flags => flags.enableOwnerStacks)) {
+        expect(ownerStack).toContain('at App');
+      } else {
+        expect(ownerStack).toBe(null);
       }
-      expect(stack).toContain('at App');
     });
 
     it('can have nested updates if they do not cross the limit', async () => {

packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js

@@ -91,15 +91,16 @@ describe('ReactIncrementalErrorLogging', () => {
           'Consider adding an error boundary to your tree ' +
             'to customize error handling behavior.',
         ),
-        expect.stringMatching(
-          new RegExp(
-            gate(flags => flags.enableOwnerStacks)
-              ? '\\s+(in|at) ErrorThrowingComponent'
-              : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
-                '\\s+(in|at) span(.*)\n' +
-                '\\s+(in|at) div(.*)',
-          ),
-        ),
+        // The component stack is not added without the polyfill/devtools.
+        // expect.stringMatching(
+        //  new RegExp(
+        //    gate(flags => flags.enableOwnerStacks)
+        //      ? '\\s+(in|at) ErrorThrowingComponent'
+        //      : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
+        //        '\\s+(in|at) span(.*)\n' +
+        //        '\\s+(in|at) div(.*)',
+        //  ),
+        // ),
       );
     }
   });
@@ -139,15 +140,16 @@ describe('ReactIncrementalErrorLogging', () => {
           'Consider adding an error boundary to your tree ' +
             'to customize error handling behavior.',
         ),
-        expect.stringMatching(
-          new RegExp(
-            gate(flags => flags.enableOwnerStacks)
-              ? '\\s+(in|at) ErrorThrowingComponent'
-              : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
-                '\\s+(in|at) span(.*)\n' +
-                '\\s+(in|at) div(.*)',
-          ),
-        ),
+        // The component stack is not added without the polyfill/devtools.
+        // expect.stringMatching(
+        //   new RegExp(
+        //     gate(flags => flags.enableOwnerStacks)
+        //       ? '\\s+(in|at) ErrorThrowingComponent'
+        //       : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
+        //         '\\s+(in|at) span(.*)\n' +
+        //         '\\s+(in|at) div(.*)',
+        //   ),
+        // ),
       );
     }
   });
@@ -199,16 +201,17 @@ describe('ReactIncrementalErrorLogging', () => {
           'React will try to recreate this component tree from scratch ' +
             'using the error boundary you provided, ErrorBoundary.',
         ),
-        expect.stringMatching(
-          new RegExp(
-            gate(flags => flags.enableOwnerStacks)
-              ? '\\s+(in|at) ErrorThrowingComponent'
-              : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
-                '\\s+(in|at) span(.*)\n' +
-                '\\s+(in|at) ErrorBoundary(.*)\n' +
-                '\\s+(in|at) div(.*)',
-          ),
-        ),
+        // The component stack is not added without the polyfill/devtools.
+        // expect.stringMatching(
+        //   new RegExp(
+        //     gate(flags => flags.enableOwnerStacks)
+        //       ? '\\s+(in|at) ErrorThrowingComponent'
+        //       : '\\s+(in|at) ErrorThrowingComponent (.*)\n' +
+        //         '\\s+(in|at) span(.*)\n' +
+        //         '\\s+(in|at) ErrorBoundary(.*)\n' +
+        //         '\\s+(in|at) div(.*)',
+        //   ),
+        // ),
       );
     } else {
       expect(logCapturedErrorCalls[0]).toEqual(
@@ -282,13 +285,14 @@ describe('ReactIncrementalErrorLogging', () => {
           'React will try to recreate this component tree from scratch ' +
             'using the error boundary you provided, ErrorBoundary.',
         ),
-        expect.stringMatching(
-          gate(flag => flag.enableOwnerStacks)
-            ? new RegExp('\\s+(in|at) Foo')
-            : new RegExp(
-                '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)',
-              ),
-        ),
+        // The component stack is not added without the polyfill/devtools.
+        // expect.stringMatching(
+        //   gate(flag => flag.enableOwnerStacks)
+        //     ? new RegExp('\\s+(in|at) Foo')
+        //     : new RegExp(
+        //         '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)',
+        //       ),
+        // ),
       );
     } else {
       expect(console.error).toHaveBeenCalledWith(