diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index 185e11461bfe4..bbe9d9a17acb6 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -2586,16 +2586,14 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 1, ⚠ 0 [root] - ✕ + `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 1, ⚠ 0 [root] - → ✕ + `); utils.act(() => unmount()); @@ -2648,16 +2646,14 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 1, ⚠ 0 [root] - ✕ + `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 1, ⚠ 0 [root] - → ✕ + `); utils.act(() => unmount()); @@ -2705,18 +2701,18 @@ describe('TreeListContext', () => { utils.act(() => TestRenderer.create()); expect(store).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] - ▾ ✕ + ▾ ✕ `); selectNextErrorOrWarning(); expect(state).toMatchInlineSnapshot(` - ✕ 2, ⚠ 0 + ✕ 1, ⚠ 0 [root] - → ▾ ✕ - ✕ + ▾ + → ✕ `); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index 65aa67d342579..9eeb4e7c072a1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -142,8 +142,8 @@ describe('ReactDOMConsoleErrorReporting', () => { // Addendum by React: expect.stringContaining('%s'), expect.stringContaining('An error occurred in the component'), - expect.stringContaining('Foo'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -207,8 +207,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -273,8 +273,8 @@ describe('ReactDOMConsoleErrorReporting', () => { // Addendum by React: expect.stringContaining('%s'), expect.stringContaining('An error occurred in the component'), - expect.stringContaining('Foo'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -343,8 +343,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -409,8 +409,8 @@ describe('ReactDOMConsoleErrorReporting', () => { // Addendum by React: expect.stringContaining('%s'), expect.stringContaining('An error occurred in the component'), - expect.stringContaining('Foo'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -477,8 +477,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js index 02e34f9322f10..65301c789d0f0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js @@ -161,8 +161,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining('%s'), // Addendum by React: expect.stringContaining('An error occurred in the component'), - expect.stringContaining('Foo'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); @@ -238,8 +238,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -307,11 +307,9 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining('%s'), // Addendum by React: - expect.stringContaining( - 'An error occurred in the component:', - ), - expect.stringContaining('Foo'), + expect.stringContaining('An error occurred in the component'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); @@ -391,8 +389,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { @@ -461,8 +459,8 @@ describe('ReactDOMConsoleErrorReporting', () => { // Addendum by React: expect.stringContaining('An error occurred in the component'), - expect.stringContaining('Foo'), expect.stringContaining('Consider adding an error boundary'), + expect.stringContaining('Foo'), ], ]); @@ -541,8 +539,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining( 'The above error occurred in the component', ), - expect.stringContaining('Foo'), expect.stringContaining('ErrorBoundary'), + expect.stringContaining('Foo'), ], ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 684f011cb6009..f45069e91539c 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -727,7 +727,7 @@ describe('ReactErrorBoundaries', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.mock.calls[0][2]).toContain( - 'The above error occurred in the component:', + 'The above error occurred in the component', ); } diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index b0b223dd43bee..2167d819cbbb8 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -705,7 +705,7 @@ describe('ReactLegacyErrorBoundaries', () => { 'ReactDOM.render has not been supported since React 18', ); expect(console.error.mock.calls[1][2]).toContain( - 'The above error occurred in the component:', + 'The above error occurred in the component', ); } diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 381346e6b4a95..a948e5c79b694 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -18,6 +18,8 @@ import reportGlobalError from 'shared/reportGlobalError'; import ReactSharedInternals from 'shared/ReactSharedInternals'; +import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; + // Side-channel since I'm not sure we want to make this part of the public API let componentName: null | string = null; let errorBoundaryName: null | string = null; @@ -34,20 +36,36 @@ export function defaultOnUncaughtError( // So we add those into a separate console.warn. reportGlobalError(error); if (__DEV__) { - const componentStack = - errorInfo.componentStack != null ? errorInfo.componentStack : ''; - const componentNameMessage = componentName - ? `An error occurred in the <${componentName}> component:` - : 'An error occurred in one of your React components:'; + ? `An error occurred in the <${componentName}> component.` + : 'An error occurred in one of your React components.'; - console['warn']( - '%s\n%s\n\n%s', - componentNameMessage, - componentStack || '', + const errorBoundaryMessage = 'Consider adding an error boundary to your tree to customize error handling behavior.\n' + - 'Visit https://react.dev/link/error-boundaries to learn more about error boundaries.', - ); + 'Visit https://react.dev/link/error-boundaries to learn more about error boundaries.'; + + if (enableOwnerStacks) { + console.warn( + '%s\n\n%s\n', + componentNameMessage, + errorBoundaryMessage, + // We let our console.error wrapper add the component stack to the end. + ); + } else { + // The current Fiber is disconnected at this point which means that console printing + // cannot add a component stack since it terminates at the deletion node. This is not + // a problem for owner stacks which are not disconnected but for the parent component + // stacks we need to use the snapshot we've previously extracted. + const componentStack = + errorInfo.componentStack != null ? errorInfo.componentStack : ''; + // Don't transform to our wrapper + console['warn']( + '%s\n\n%s\n%s', + componentNameMessage, + errorBoundaryMessage, + componentStack, + ); + } } } @@ -63,31 +81,47 @@ export function defaultOnCaughtError( // Caught by error boundary if (__DEV__) { - const componentStack = - errorInfo.componentStack != null ? errorInfo.componentStack : ''; - const componentNameMessage = componentName - ? `The above error occurred in the <${componentName}> component:` - : 'The above error occurred in one of your React components:'; + ? `The above error occurred in the <${componentName}> component.` + : 'The above error occurred in one of your React components.'; // In development, we provide our own message which includes the component stack // in addition to the error. - // Don't transform to our wrapper - console['error']( - '%o\n\n%s\n%s\n\n%s', - error, - componentNameMessage, - componentStack, + const recreateMessage = `React will try to recreate this component tree from scratch ` + - `using the error boundary you provided, ${ - errorBoundaryName || 'Anonymous' - }.`, - ); + `using the error boundary you provided, ${ + errorBoundaryName || 'Anonymous' + }.`; + + if (enableOwnerStacks) { + console.error( + '%o\n\n%s\n\n%s\n', + error, + componentNameMessage, + recreateMessage, + // We let our consoleWithStackDev wrapper add the component stack to the end. + ); + } else { + // The current Fiber is disconnected at this point which means that console printing + // cannot add a component stack since it terminates at the deletion node. This is not + // a problem for owner stacks which are not disconnected but for the parent component + // stacks we need to use the snapshot we've previously extracted. + const componentStack = + errorInfo.componentStack != null ? errorInfo.componentStack : ''; + // Don't transform to our wrapper + console['error']( + '%o\n\n%s\n\n%s\n%s', + error, + componentNameMessage, + recreateMessage, + componentStack, + ); + } } else { // In production, we print the error directly. // This will include the message, the JS stack, and anything the browser wants to show. // We pass the error object instead of custom message so that the browser displays the error natively. - console['error'](error); // Don't transform to our wrapper + console['error'](error); // Don't transform to our wrapper, however, React DevTools can still add a stack. } } diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 8393e3d007a23..40a68b1c047b0 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -87,6 +87,10 @@ import { import {ConcurrentRoot} from './ReactRootTags'; import {noopSuspenseyCommitThenable} from './ReactFiberThenable'; import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols'; +import { + setCurrentDebugFiberInDEV, + getCurrentFiber as getCurrentDebugFiberInDEV, +} from './ReactCurrentFiber'; function createRootErrorUpdate( root: FiberRoot, @@ -100,7 +104,10 @@ function createRootErrorUpdate( // being called "element". update.payload = {element: null}; update.callback = () => { + const prevFiber = getCurrentDebugFiberInDEV(); // should just be the root + setCurrentDebugFiberInDEV(errorInfo.source); logUncaughtError(root, errorInfo); + setCurrentDebugFiberInDEV(prevFiber); }; return update; } @@ -127,7 +134,10 @@ function initializeClassErrorUpdate( if (__DEV__) { markFailedErrorBoundaryForHotReloading(fiber); } + const prevFiber = getCurrentDebugFiberInDEV(); // should be the error boundary + setCurrentDebugFiberInDEV(errorInfo.source); logCaughtError(root, fiber, errorInfo); + setCurrentDebugFiberInDEV(prevFiber); }; } @@ -138,7 +148,10 @@ function initializeClassErrorUpdate( if (__DEV__) { markFailedErrorBoundaryForHotReloading(fiber); } + const prevFiber = getCurrentDebugFiberInDEV(); // should be the error boundary + setCurrentDebugFiberInDEV(errorInfo.source); logCaughtError(root, fiber, errorInfo); + setCurrentDebugFiberInDEV(prevFiber); if (typeof getDerivedStateFromError !== 'function') { // To preserve the preexisting retry behavior of error boundaries, // we keep track of which ones already failed during this batch. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index e46927c1bc498..9c0a3d3f15c07 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3029,7 +3029,9 @@ function commitRootImpl( for (let i = 0; i < recoverableErrors.length; i++) { const recoverableError = recoverableErrors[i]; const errorInfo = makeErrorInfo(recoverableError.stack); + setCurrentDebugFiberInDEV(recoverableError.source); onRecoverableError(recoverableError.value, errorInfo); + resetCurrentDebugFiberInDEV(); } } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index c6e342871b198..b0ac81016e93b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1512,7 +1512,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.mock.calls[0][1]).toBe(notAnError); expect(console.error.mock.calls[0][2]).toContain( - 'The above error occurred in the component:', + 'The above error occurred in the component', ); } else { expect(console.error).toHaveBeenCalledTimes(1); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index d4c7949580c97..3bd934af38da6 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -85,7 +85,11 @@ describe('ReactIncrementalErrorLogging', () => { expect(console.warn).toHaveBeenCalledWith( expect.stringContaining('%s'), expect.stringContaining( - 'An error occurred in the component:', + 'An error occurred in the component.', + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', ), expect.stringMatching( new RegExp( @@ -94,10 +98,6 @@ describe('ReactIncrementalErrorLogging', () => { '\\s+(in|at) div(.*)', ), ), - expect.stringContaining( - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior.', - ), ); } }); @@ -131,7 +131,11 @@ describe('ReactIncrementalErrorLogging', () => { expect(console.warn).toHaveBeenCalledWith( expect.stringContaining('%s'), expect.stringContaining( - 'An error occurred in the component:', + 'An error occurred in the component.', + ), + expect.stringContaining( + 'Consider adding an error boundary to your tree ' + + 'to customize error handling behavior.', ), expect.stringMatching( new RegExp( @@ -140,10 +144,6 @@ describe('ReactIncrementalErrorLogging', () => { '\\s+(in|at) div(.*)', ), ), - expect.stringContaining( - 'Consider adding an error boundary to your tree ' + - 'to customize error handling behavior.', - ), ); } }); @@ -189,7 +189,11 @@ describe('ReactIncrementalErrorLogging', () => { message: 'render error', }), expect.stringContaining( - 'The above error occurred in the component:', + 'The above error occurred in the component.', + ), + expect.stringContaining( + 'React will try to recreate this component tree from scratch ' + + 'using the error boundary you provided, ErrorBoundary.', ), expect.stringMatching( new RegExp( @@ -199,10 +203,6 @@ describe('ReactIncrementalErrorLogging', () => { '\\s+(in|at) div(.*)', ), ), - expect.stringContaining( - 'React will try to recreate this component tree from scratch ' + - 'using the error boundary you provided, ErrorBoundary.', - ), ); } else { expect(logCapturedErrorCalls[0]).toEqual( @@ -270,17 +270,21 @@ describe('ReactIncrementalErrorLogging', () => { message: 'oops', }), expect.stringContaining( - 'The above error occurred in the component:', - ), - expect.stringMatching( - new RegExp( - '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)', - ), + 'The above error occurred in the component.', ), expect.stringContaining( 'React will try to recreate this component tree from scratch ' + 'using the error boundary you provided, ErrorBoundary.', ), + expect.stringMatching( + gate(flag => flag.enableOwnerStacks) + ? // With owner stacks the return path is cut off but in this case + // this is also what the owner stack looks like. + new RegExp('\\s+(in|at) Foo (.*)') + : new RegExp( + '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)', + ), + ), ); } else { expect(console.error).toHaveBeenCalledWith( diff --git a/packages/shared/consoleWithStackDev.js b/packages/shared/consoleWithStackDev.js index d815d8796d809..1061aaf1821c1 100644 --- a/packages/shared/consoleWithStackDev.js +++ b/packages/shared/consoleWithStackDev.js @@ -6,6 +6,7 @@ */ import ReactSharedInternals from 'shared/ReactSharedInternals'; +import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; let suppressWarning = false; export function setSuppressWarning(newSuppressWarning) { @@ -40,19 +41,30 @@ function printWarning(level, format, args) { // When changing this logic, you might want to also // update consoleWithStackDev.www.js as well. if (__DEV__) { + const isErrorLogger = + format === '%s\n\n%s\n' || format === '%o\n\n%s\n\n%s\n'; + const stack = ReactSharedInternals.getStackAddendum(); if (stack !== '') { format += '%s'; args = args.concat([stack]); } - // eslint-disable-next-line react-internal/safe-string-coercion - const argsWithFormat = args.map(item => String(item)); - // Careful: RN currently depends on this prefix - argsWithFormat.unshift('Warning: ' + format); + if (isErrorLogger) { + // Don't prefix our default logging formatting in ReactFiberErrorLoggger. + // Don't toString the arguments. + args.unshift(format); + } else { + // TODO: Remove this prefix and stop toStringing in the wrapper and + // instead do it at each callsite as needed. + // Careful: RN currently depends on this prefix + // eslint-disable-next-line react-internal/safe-string-coercion + args = args.map(item => String(item)); + args.unshift('Warning: ' + format); + } // We intentionally don't use spread (or .apply) directly because it // breaks IE9: https://github.com/facebook/react/issues/13610 // eslint-disable-next-line react-internal/no-production-logging - Function.prototype.apply.call(console[level], console, argsWithFormat); + Function.prototype.apply.call(console[level], console, args); } } diff --git a/scripts/eslint-rules/warning-args.js b/scripts/eslint-rules/warning-args.js index 701d4db267915..47df4e519a9c5 100644 --- a/scripts/eslint-rules/warning-args.js +++ b/scripts/eslint-rules/warning-args.js @@ -73,7 +73,10 @@ module.exports = { ); return; } - if (format.length < 10 || /^[s\W]*$/.test(format)) { + if ( + (format.length < 10 || /^[s\W]*$/.test(format)) && + format !== '%s\n\n%s\n' + ) { context.report( node, 'The {{name}} format should be able to uniquely identify this ' + @@ -83,7 +86,7 @@ module.exports = { return; } // count the number of formatting substitutions, plus the first two args - const expectedNArgs = (format.match(/%s/g) || []).length + 1; + const expectedNArgs = (format.match(/%[so]/g) || []).length + 1; if (node.arguments.length !== expectedNArgs) { context.report( node,