Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function getCurrentParentStackInDev(): string {
return '';
}

function getCurrentFiberStackInDev(): string {
function getCurrentFiberStackInDev(stack: Error): string {
if (__DEV__) {
if (current === null) {
return '';
Expand All @@ -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);
}
Expand Down
22 changes: 15 additions & 7 deletions packages/react-reconciler/src/ReactFiberCallUserSpace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -20,10 +20,14 @@ export function callComponentInDEV<Props, Arg, R>(
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<R> {
Expand All @@ -32,10 +36,14 @@ interface ClassInstance<R> {

/** @noinline */
export function callRenderInDEV<R>(instance: ClassInstance<R>): 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 */
Expand Down
22 changes: 19 additions & 3 deletions packages/react-reconciler/src/ReactFiberComponentStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
);
Expand Down
5 changes: 5 additions & 0 deletions packages/react-reconciler/src/ReactFiberOwnerStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1618,8 +1618,7 @@ describe('ReactHooks', () => {
' Previous render Next render\n' +
' ------------------------------------------------------\n' +
`1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameB)}\n` +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' +
' in App (at **)',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the stack frame still available in browsers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these matches a subset, by removing the stack frame it matches any stack frame.

These end up having the mentioned internals in them so I didn't want to assert on those.

' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);

// further warnings for this component are silenced
Expand Down Expand Up @@ -1671,8 +1670,7 @@ describe('ReactHooks', () => {
' ------------------------------------------------------\n' +
`1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameA)}\n` +
`2. undefined use${hookNameB}\n` +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' +
' in App (at **)',
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);
});
});
Expand Down Expand Up @@ -1758,8 +1756,7 @@ describe('ReactHooks', () => {
'ImperativeHandle',
'Memo',
)}\n` +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' +
' in App (at **)',
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);

// further warnings for this component are silenced
Expand Down
16 changes: 4 additions & 12 deletions packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down
6 changes: 4 additions & 2 deletions packages/react/src/ReactSharedInternalsClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type SharedStateClient = {
thrownErrors: Array<mixed>,

// ReactDebugCurrentFrame
getCurrentStack: null | (() => string),
getCurrentStack: null | ((stack: Error) => string),
};

export type RendererTask = boolean => RendererTask | null;
Expand All @@ -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;
6 changes: 4 additions & 2 deletions packages/react/src/ReactSharedInternalsServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type SharedStateServer = {
// DEV-only

// ReactDebugCurrentFrame
getCurrentStack: null | (() => string),
getCurrentStack: null | ((stack: Error) => string),
};

export type RendererTask = boolean => RendererTask | null;
Expand All @@ -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;
8 changes: 4 additions & 4 deletions packages/shared/consoleWithStackDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,23 @@ 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'));
}
}
}

export function error(format, ...args) {
if (__DEV__) {
if (!suppressWarning) {
printWarning('error', format, args);
printWarning('error', format, args, new Error('react-stack-top-frame'));
}
}
}

// 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__) {
Expand All @@ -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]);
Expand Down
8 changes: 4 additions & 4 deletions packages/shared/forks/consoleWithStackDev.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,27 @@ 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'));
}
}
}

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);
Expand Down