diff --git a/packages/react-reconciler/src/ReactCurrentFiber.js b/packages/react-reconciler/src/ReactCurrentFiber.js index cf0c2543a57..fd2b4e7d802 100644 --- a/packages/react-reconciler/src/ReactCurrentFiber.js +++ b/packages/react-reconciler/src/ReactCurrentFiber.js @@ -44,7 +44,7 @@ export function getCurrentParentStackInDev(): string { return ''; } -function getCurrentFiberStackInDev(): string { +function getCurrentFiberStackInDev(stack: Error): string { if (__DEV__) { if (current === null) { return ''; @@ -54,7 +54,7 @@ function getCurrentFiberStackInDev(): string { // TODO: The above comment is not actually true. We might be // in a commit phase or preemptive set state callback. if (enableOwnerStacks) { - return getOwnerStackByFiberInDev(current); + return getOwnerStackByFiberInDev(current, stack); } return getStackByFiberInDevAndProd(current); } diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index dfc88b64be8..e85d0431b99 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -9,7 +9,7 @@ import type {LazyComponent} from 'react/src/ReactLazy'; -import {setIsRendering} from './ReactCurrentFiber'; +import {isRendering, setIsRendering} from './ReactCurrentFiber'; // These indirections exists so we can exclude its stack frame in DEV (and anything below it). // TODO: Consider marking the whole bundle instead of these boundaries. @@ -20,10 +20,14 @@ export function callComponentInDEV( props: Props, secondArg: Arg, ): R { + const wasRendering = isRendering; setIsRendering(true); - const result = Component(props, secondArg); - setIsRendering(false); - return result; + try { + const result = Component(props, secondArg); + return result; + } finally { + setIsRendering(wasRendering); + } } interface ClassInstance { @@ -32,10 +36,14 @@ interface ClassInstance { /** @noinline */ export function callRenderInDEV(instance: ClassInstance): R { + const wasRendering = isRendering; setIsRendering(true); - const result = instance.render(); - setIsRendering(false); - return result; + try { + const result = instance.render(); + return result; + } finally { + setIsRendering(wasRendering); + } } /** @noinline */ diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js index e5e25f67469..8a69ba9ddfd 100644 --- a/packages/react-reconciler/src/ReactFiberComponentStack.js +++ b/packages/react-reconciler/src/ReactFiberComponentStack.js @@ -90,13 +90,27 @@ function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string { return name ? describeBuiltInComponentFrame(name) : ''; } -export function getOwnerStackByFiberInDev(workInProgress: Fiber): string { +export function getOwnerStackByFiberInDev( + workInProgress: Fiber, + topStack: null | Error, +): string { if (!enableOwnerStacks || !__DEV__) { return ''; } try { let info = ''; + if (topStack) { + // Prefix with a filtered version of the currently executing + // stack. This information will be available in the native + // stack regardless but it's hidden since we're reprinting + // the stack on top of it. + const formattedTopStack = formatOwnerStack(topStack); + if (formattedTopStack !== '') { + info += '\n' + formattedTopStack; + } + } + if (workInProgress.tag === HostText) { // Text nodes never have an owner/stack because they're not created through JSX. // We use the parent since text nodes are always created through a host parent. @@ -125,14 +139,16 @@ export function getOwnerStackByFiberInDev(workInProgress: Fiber): string { case FunctionComponent: case SimpleMemoComponent: case ClassComponent: - if (!workInProgress._debugOwner) { + if (!workInProgress._debugOwner && info === '') { + // Only if we have no other data about the callsite do we add + // the component name as the single stack frame. info += describeFunctionComponentFrameWithoutLineNumber( workInProgress.type, ); } break; case ForwardRef: - if (!workInProgress._debugOwner) { + if (!workInProgress._debugOwner && info === '') { info += describeFunctionComponentFrameWithoutLineNumber( workInProgress.type.render, ); diff --git a/packages/react-reconciler/src/ReactFiberOwnerStack.js b/packages/react-reconciler/src/ReactFiberOwnerStack.js index b8510ebf6bc..fe9e4f1cfd6 100644 --- a/packages/react-reconciler/src/ReactFiberOwnerStack.js +++ b/packages/react-reconciler/src/ReactFiberOwnerStack.js @@ -103,6 +103,11 @@ function filterDebugStack(error: Error): string { if (lastFrameIdx !== -1) { // Cut off everything after our "callComponent" slot since it'll be Fiber internals. frames.length = lastFrameIdx; + } else { + // We didn't find any internal callsite out to user space. + // This means that this was called outside an owner or the owner is fully internal. + // To keep things light we exclude the entire trace in this case. + return ''; } return frames.filter(isNotExternal).join('\n'); } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 8cc5c15a9e0..fd1879dabfb 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1618,8 +1618,7 @@ describe('ReactHooks', () => { ' Previous render Next render\n' + ' ------------------------------------------------------\n' + `1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameB)}\n` + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + - ' in App (at **)', + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', ]); // further warnings for this component are silenced @@ -1671,8 +1670,7 @@ describe('ReactHooks', () => { ' ------------------------------------------------------\n' + `1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameA)}\n` + `2. undefined use${hookNameB}\n` + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + - ' in App (at **)', + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', ]); }); }); @@ -1758,8 +1756,7 @@ describe('ReactHooks', () => { 'ImperativeHandle', 'Memo', )}\n` + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + - ' in App (at **)', + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', ]); // further warnings for this component are silenced diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index 3ec58b2f705..73f9aa9cc5d 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -228,18 +228,10 @@ describe('ReactLazy', () => { expect(error.message).toMatch('Element type is invalid'); assertLog(['Loading...']); - assertConsoleErrorDev( - [ - 'Expected the result of a dynamic import() call', - 'Expected the result of a dynamic import() call', - ], - gate(flags => flags.enableOwnerStacks) - ? { - // There's no owner - withoutStack: true, - } - : undefined, - ); + assertConsoleErrorDev([ + 'Expected the result of a dynamic import() call', + 'Expected the result of a dynamic import() call', + ]); expect(root).not.toMatchRenderedOutput('Hi'); }); diff --git a/packages/react/src/ReactSharedInternalsClient.js b/packages/react/src/ReactSharedInternalsClient.js index 452bd933dab..6a54c73be30 100644 --- a/packages/react/src/ReactSharedInternalsClient.js +++ b/packages/react/src/ReactSharedInternalsClient.js @@ -35,7 +35,7 @@ export type SharedStateClient = { thrownErrors: Array, // ReactDebugCurrentFrame - getCurrentStack: null | (() => string), + getCurrentStack: null | ((stack: Error) => string), }; export type RendererTask = boolean => RendererTask | null; @@ -54,7 +54,9 @@ if (__DEV__) { ReactSharedInternals.didUsePromise = false; ReactSharedInternals.thrownErrors = []; // Stack implementation injected by the current renderer. - ReactSharedInternals.getCurrentStack = (null: null | (() => string)); + ReactSharedInternals.getCurrentStack = (null: + | null + | ((stack: Error) => string)); } export default ReactSharedInternals; diff --git a/packages/react/src/ReactSharedInternalsServer.js b/packages/react/src/ReactSharedInternalsServer.js index d670fa18fe7..749ce5c3ad4 100644 --- a/packages/react/src/ReactSharedInternalsServer.js +++ b/packages/react/src/ReactSharedInternalsServer.js @@ -38,7 +38,7 @@ export type SharedStateServer = { // DEV-only // ReactDebugCurrentFrame - getCurrentStack: null | (() => string), + getCurrentStack: null | ((stack: Error) => string), }; export type RendererTask = boolean => RendererTask | null; @@ -58,7 +58,9 @@ if (enableTaint) { if (__DEV__) { // Stack implementation injected by the current renderer. - ReactSharedInternals.getCurrentStack = (null: null | (() => string)); + ReactSharedInternals.getCurrentStack = (null: + | null + | ((stack: Error) => string)); } export default ReactSharedInternals; diff --git a/packages/shared/consoleWithStackDev.js b/packages/shared/consoleWithStackDev.js index bdcf7548021..4638ede81cb 100644 --- a/packages/shared/consoleWithStackDev.js +++ b/packages/shared/consoleWithStackDev.js @@ -24,7 +24,7 @@ export function setSuppressWarning(newSuppressWarning) { export function warn(format, ...args) { if (__DEV__) { if (!suppressWarning) { - printWarning('warn', format, args); + printWarning('warn', format, args, new Error('react-stack-top-frame')); } } } @@ -32,7 +32,7 @@ export function warn(format, ...args) { export function error(format, ...args) { if (__DEV__) { if (!suppressWarning) { - printWarning('error', format, args); + printWarning('error', format, args, new Error('react-stack-top-frame')); } } } @@ -40,7 +40,7 @@ export function error(format, ...args) { // eslint-disable-next-line react-internal/no-production-logging const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask; -function printWarning(level, format, args) { +function printWarning(level, format, args, currentStack) { // When changing this logic, you might want to also // update consoleWithStackDev.www.js as well. if (__DEV__) { @@ -51,7 +51,7 @@ function printWarning(level, format, args) { // 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(); + const stack = ReactSharedInternals.getCurrentStack(currentStack); if (stack !== '') { format += '%s'; args = args.concat([stack]); diff --git a/packages/shared/forks/consoleWithStackDev.www.js b/packages/shared/forks/consoleWithStackDev.www.js index c4311efe09f..5f04f36359f 100644 --- a/packages/shared/forks/consoleWithStackDev.www.js +++ b/packages/shared/forks/consoleWithStackDev.www.js @@ -18,7 +18,7 @@ export function setSuppressWarning(newSuppressWarning) { export function warn(format, ...args) { if (__DEV__) { if (!suppressWarning) { - printWarning('warn', format, args); + printWarning('warn', format, args, new Error('react-stack-top-frame')); } } } @@ -26,19 +26,19 @@ export function warn(format, ...args) { export function error(format, ...args) { if (__DEV__) { if (!suppressWarning) { - printWarning('error', format, args); + printWarning('error', format, args, new Error('react-stack-top-frame')); } } } -function printWarning(level, format, args) { +function printWarning(level, format, args, currentStack) { if (__DEV__) { const React = require('react'); const ReactSharedInternals = React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE; // Defensive in case this is fired before React is initialized. if (ReactSharedInternals != null && ReactSharedInternals.getCurrentStack) { - const stack = ReactSharedInternals.getCurrentStack(); + const stack = ReactSharedInternals.getCurrentStack(currentStack); if (stack !== '') { format += '%s'; args.push(stack);