diff --git a/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js b/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js index ad3a7cdce7cae..befbdc617d361 100644 --- a/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js +++ b/packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js @@ -866,16 +866,40 @@ describe('ReactInternalTestUtils console assertions', () => { const message = expectToThrowFailure(() => { expect(root).toMatchRenderedOutput(
foobarbaz
); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.log was called without assertConsoleLogDev: - + Not asserted - + Not asserted - + Not asserted + console.log was called without assertConsoleLogDev: + + Not asserted + + Not asserted + + Not asserted - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + Not asserted + + Not asserted + + Not asserted + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + Not asserted + + Not asserted + + Not asserted + + You must call one of the assertConsoleDev helpers between each act call." + `); + } expect(root).toMatchRenderedOutput(
foobarbaz
); }); @@ -922,16 +946,52 @@ describe('ReactInternalTestUtils console assertions', () => { }); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.warn was called without assertConsoleWarnDev: - + A - + B - + C + console.warn was called without assertConsoleWarnDev: + + A + + B + + C - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } }); it('fails if act is called without any assertConsoleDev helpers', async () => { @@ -962,26 +1022,94 @@ describe('ReactInternalTestUtils console assertions', () => { }); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.log was called without assertConsoleLogDev: - + A - + B - + C + console.log was called without assertConsoleLogDev: + + A + + B + + C - console.warn was called without assertConsoleWarnDev: - + A - + B - + C + console.warn was called without assertConsoleWarnDev: + + A + + B + + C - console.error was called without assertConsoleErrorDev: - + A - + B - + C + console.error was called without assertConsoleErrorDev: + + A + + B + + C - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + A + + B + + C + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + A + + B + + C + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } }); // @gate __DEV__ @@ -1804,16 +1932,49 @@ describe('ReactInternalTestUtils console assertions', () => { const message = expectToThrowFailure(() => { expect(root).toMatchRenderedOutput(
foobarbaz
); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.warn was called without assertConsoleWarnDev: - + Not asserted - + Not asserted - + Not asserted + console.warn was called without assertConsoleWarnDev: + + Not asserted + + Not asserted + + Not asserted - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.warn was called without assertConsoleWarnDev: + + Not asserted%s, + + in Yield (at **) + + Not asserted%s, + + in Yield (at **) + + Not asserted%s, + + in Yield (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.warn was called without assertConsoleWarnDev: + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } expect(root).toMatchRenderedOutput(
foobarbaz
); }); @@ -1860,16 +2021,52 @@ describe('ReactInternalTestUtils console assertions', () => { }); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.error was called without assertConsoleErrorDev: - + A - + B - + C + console.error was called without assertConsoleErrorDev: + + A + + B + + C - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } }); it('fails if act is called without any assertConsoleDev helpers', async () => { @@ -1900,26 +2097,94 @@ describe('ReactInternalTestUtils console assertions', () => { }); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.log was called without assertConsoleLogDev: - + A - + B - + C + console.log was called without assertConsoleLogDev: + + A + + B + + C - console.warn was called without assertConsoleWarnDev: - + A - + B - + C + console.warn was called without assertConsoleWarnDev: + + A + + B + + C - console.error was called without assertConsoleErrorDev: - + A - + B - + C + console.error was called without assertConsoleErrorDev: + + A + + B + + C - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + A + + B + + C + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in App (at **) + + B%s, + + in App (at **) + + C%s, + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.log was called without assertConsoleLogDev: + + A + + B + + C + + console.warn was called without assertConsoleWarnDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + console.error was called without assertConsoleErrorDev: + + A%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + B%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + C%s, + + in Yield (at **) + + in div (at **) + + in App (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } }); // @gate __DEV__ @@ -2786,16 +3051,49 @@ describe('ReactInternalTestUtils console assertions', () => { const message = expectToThrowFailure(() => { expect(root).toMatchRenderedOutput(
foobarbaz
); }); - expect(message).toMatchInlineSnapshot(` - "asserConsoleLogsCleared(expected) + if (!__DEV__) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) - console.error was called without assertConsoleErrorDev: - + Not asserted - + Not asserted - + Not asserted + console.error was called without assertConsoleErrorDev: + + Not asserted + + Not asserted + + Not asserted - You must call one of the assertConsoleDev helpers between each act call." - `); + You must call one of the assertConsoleDev helpers between each act call." + `); + } else if (gate(flags => flags.enableOwnerStacks)) { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.error was called without assertConsoleErrorDev: + + Not asserted%s, + + in Yield (at **) + + Not asserted%s, + + in Yield (at **) + + Not asserted%s, + + in Yield (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } else { + expect(message).toMatchInlineSnapshot(` + "asserConsoleLogsCleared(expected) + + console.error was called without assertConsoleErrorDev: + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + Not asserted%s, + + in Yield (at **) + + in div (at **) + + You must call one of the assertConsoleDev helpers between each act call." + `); + } expect(root).toMatchRenderedOutput(
foobarbaz
); }); diff --git a/packages/internal-test-utils/consoleMock.js b/packages/internal-test-utils/consoleMock.js index 3f35d9e12243a..6d03c74c1dfee 100644 --- a/packages/internal-test-utils/consoleMock.js +++ b/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('') > -1 || diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index af56b8c1a276d..e48c926a9111a 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/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
; + 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( -
{Array(6).fill()}
, + 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', () => { '
Womp womp: {Error}
\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}, ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index 9eeb4e7c072a1..2aca1e0588462 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -143,7 +143,8 @@ describe('ReactDOMConsoleErrorReporting', () => { expect.stringContaining('%s'), expect.stringContaining('An error occurred in the 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 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 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 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 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 component', ), expect.stringContaining('ErrorBoundary'), - expect.stringContaining('Foo'), + // The component stack is not added without the polyfill/devtools. + // 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 65301c789d0f0..99bfeac56b1d3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js @@ -162,7 +162,8 @@ describe('ReactDOMConsoleErrorReporting', () => { // Addendum by React: expect.stringContaining('An error occurred in the 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 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 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 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 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 component', ), expect.stringContaining('ErrorBoundary'), - expect.stringContaining('Foo'), + // The component stack is not added without the polyfill/devtools. + // expect.stringContaining('Foo'), ], ]); } else { diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 1d71c71476744..4916a7212937d 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/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 () => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index 8333445c69784..6aa4034636c31 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/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( diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index a0d4b9e140566..1eaf5369c79c8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -99,8 +99,6 @@ import {resolveOwner, setCurrentOwner} from './flight/ReactFlightCurrentOwner'; import {getOwnerStackByComponentInfoInDev} from './flight/ReactFlightComponentStack'; -import {isWritingAppendedStack} from 'shared/consoleWithStackDev'; - import { getIteratorFn, REACT_ELEMENT_TYPE, @@ -267,9 +265,8 @@ function patchConsole(consoleInst: typeof console, methodName: string) { 'name', ); const wrapperMethod = function (this: typeof console) { - let args = arguments; const request = resolveRequest(); - if (methodName === 'assert' && args[0]) { + if (methodName === 'assert' && arguments[0]) { // assert doesn't emit anything unless first argument is falsy so we can skip it. } else if (request !== null) { // Extract the stack. Not all console logs print the full stack but they have at @@ -281,22 +278,7 @@ function patchConsole(consoleInst: typeof console, methodName: string) { // refer to previous logs in debug info to associate them with a component. const id = request.nextChunkId++; const owner: null | ReactComponentInfo = resolveOwner(); - if ( - isWritingAppendedStack && - (methodName === 'error' || methodName === 'warn') && - args.length > 1 && - typeof args[0] === 'string' && - args[0].endsWith('%s') - ) { - // This looks like we've appended the component stack to the error from our own logs. - // We don't want those added to the replayed logs since those have the opportunity to add - // their own stacks or use console.createTask on the client as needed. - // TODO: Remove this special case once we remove consoleWithStackDev. - // $FlowFixMe[method-unbinding] - args = Array.prototype.slice.call(args, 0, args.length - 1); - args[0] = args[0].slice(0, args[0].length - 2); - } - emitConsoleChunk(request, id, methodName, owner, stack, args); + emitConsoleChunk(request, id, methodName, owner, stack, arguments); } // $FlowFixMe[prop-missing] return originalMethod.apply(this, arguments); diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index 330150506c384..811d897e859e7 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -804,7 +804,7 @@ describe('create-react-class-integration', () => { 'MyComponent: isMounted is deprecated. Instead, make sure to ' + 'clean up subscriptions and pending requests in componentWillUnmount ' + 'to prevent memory leaks.', - {withoutStack: true}, + // This now has a component stack even though it's part of a third-party library. ); // Dedupe diff --git a/packages/shared/consoleWithStackDev.js b/packages/shared/consoleWithStackDev.js index f709ca1e1fb98..b978b0b1d0804 100644 --- a/packages/shared/consoleWithStackDev.js +++ b/packages/shared/consoleWithStackDev.js @@ -5,9 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; - export function setSuppressWarning(newSuppressWarning) { // TODO: Noop. Delete. } @@ -19,39 +16,25 @@ export function setSuppressWarning(newSuppressWarning) { // they are left as they are instead. export function warn(format, ...args) { - printWarning('warn', format, args, new Error('react-stack-top-frame')); + if (__DEV__) { + printWarning('warn', format, args); + } } export function error(format, ...args) { - printWarning('error', format, args, new Error('react-stack-top-frame')); + if (__DEV__) { + printWarning('error', format, args); + } } -// eslint-disable-next-line react-internal/no-production-logging -const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask; - -export let isWritingAppendedStack = false; - -function printWarning(level, format, args, currentStack) { +function printWarning(level, format, args) { // When changing this logic, you might want to also // update consoleWithStackDev.www.js as well. if (__DEV__) { - if (!supportsCreateTask && ReactSharedInternals.getCurrentStack) { - // We only add the current stack to the console when createTask is not supported. - // Since createTask requires DevTools to be open to work, this means that stacks - // can be lost while DevTools isn't open but we can't detect this. - const stack = ReactSharedInternals.getCurrentStack(currentStack); - if (stack !== '') { - isWritingAppendedStack = true; - format += '%s'; - args = args.concat([stack]); - } - } - args.unshift(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, args); - isWritingAppendedStack = false; } } diff --git a/packages/shared/forks/consoleWithStackDev.rn.js b/packages/shared/forks/consoleWithStackDev.rn.js index 44767992cdd86..6ac30b39fb384 100644 --- a/packages/shared/forks/consoleWithStackDev.rn.js +++ b/packages/shared/forks/consoleWithStackDev.rn.js @@ -36,14 +36,11 @@ export function error(format, ...args) { } } -export let isWritingAppendedStack = false; - function printWarning(level, format, args) { if (__DEV__) { if (ReactSharedInternals.getCurrentStack) { const stack = ReactSharedInternals.getCurrentStack(); if (stack !== '') { - isWritingAppendedStack = true; format += '%s'; args = args.concat([stack]); } @@ -54,6 +51,5 @@ function printWarning(level, format, args) { // 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, args); - isWritingAppendedStack = false; } } diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 1114b65dfa821..b93a4daf75da5 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -80,6 +80,34 @@ const createMatcherFor = (consoleMethod, matcherName) => return; } + // Append Component Stacks. Simulates a framework or DevTools appending them. + if ( + typeof format === 'string' && + (consoleMethod === 'error' || consoleMethod === '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); + } + } + } + } + const message = util.format(format, ...args); const normalizedMessage = normalizeCodeLocInfo(message);