Remove top stack frame from getCurrentStack (#30306)

The full stack is the current execution stack (`new Error().stack`) +
the current owner stack (`React.captureOwnerStack()`).

The idea with the top frame was that when we append it to console.error
we'd include both since otherwise the true reason would be obscured
behind the little `>` to expand. So we'd just put both stack front and
center. By adding this into getCurrentStack it was easy to use the same
filtering. I never implemented in Fizz or Flight though.

However, with the public API `React.captureOwnerStack()` it's not
necessary to include the current stack since you already have it and
you'd have filtering capabilities in user space too.

Since I'm removing the component stacks from React itself we no longer
need this. It's expected that maybe RDT or framework polyfill would
include this same technique though.
This commit is contained in:
Sebastian Markbåge 2024-07-11 18:34:41 -04:00 committed by GitHub
parent af28f480e8
commit 433068eece
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 25 additions and 39 deletions

View File

@ -33,7 +33,7 @@ export function getCurrentFiberOwnerNameInDevOrNull(): string | null {
return null;
}
function getCurrentFiberStackInDev(stack: null | Error): string {
function getCurrentFiberStackInDev(): string {
if (__DEV__) {
if (current === null) {
return '';
@ -43,7 +43,7 @@ function getCurrentFiberStackInDev(stack: null | Error): 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, stack);
return getOwnerStackByFiberInDev(current);
}
return getStackByFiberInDevAndProd(current);
}

View File

@ -91,27 +91,13 @@ function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
return name ? describeBuiltInComponentFrame(name) : '';
}
export function getOwnerStackByFiberInDev(
workInProgress: Fiber,
topStack: null | Error,
): string {
export function getOwnerStackByFiberInDev(workInProgress: Fiber): 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.

View File

@ -213,14 +213,18 @@ describe('ReactLazy', () => {
unstable_isConcurrent: true,
});
function App() {
return (
<Suspense fallback={<Text text="Loading..." />}>
<LazyText text="Hi" />
</Suspense>
);
}
let error;
try {
await act(() => {
root.update(
<Suspense fallback={<Text text="Loading..." />}>
<LazyText text="Hi" />
</Suspense>,
);
root.update(<App />);
});
} catch (e) {
error = e;

View File

@ -20,5 +20,5 @@ export function captureOwnerStack(): null | string {
}
// The current stack will be the owner stack if enableOwnerStacks is true
// which it is always here. Otherwise it's the parent stack.
return getCurrentStack(null);
return getCurrentStack();
}

View File

@ -35,7 +35,7 @@ export type SharedStateClient = {
thrownErrors: Array<mixed>,
// ReactDebugCurrentFrame
getCurrentStack: null | ((stack: null | Error) => string),
getCurrentStack: null | (() => string),
};
export type RendererTask = boolean => RendererTask | null;
@ -54,9 +54,7 @@ if (__DEV__) {
ReactSharedInternals.didUsePromise = false;
ReactSharedInternals.thrownErrors = [];
// Stack implementation injected by the current renderer.
ReactSharedInternals.getCurrentStack = (null:
| null
| ((stack: null | Error) => string));
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
}
export default ReactSharedInternals;

View File

@ -38,7 +38,7 @@ export type SharedStateServer = {
// DEV-only
// ReactDebugCurrentFrame
getCurrentStack: null | ((stack: null | Error) => string),
getCurrentStack: null | (() => string),
};
export type RendererTask = boolean => RendererTask | null;
@ -58,9 +58,7 @@ if (enableTaint) {
if (__DEV__) {
// Stack implementation injected by the current renderer.
ReactSharedInternals.getCurrentStack = (null:
| null
| ((stack: null | Error) => string));
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
}
export default ReactSharedInternals;

View File

@ -23,7 +23,7 @@ export function setSuppressWarning(newSuppressWarning) {
export function warn(format, ...args) {
if (__DEV__) {
if (!suppressWarning) {
printWarning('warn', format, args, new Error('react-stack-top-frame'));
printWarning('warn', format, args);
}
}
}
@ -31,17 +31,17 @@ export function warn(format, ...args) {
export function error(format, ...args) {
if (__DEV__) {
if (!suppressWarning) {
printWarning('error', format, args, new Error('react-stack-top-frame'));
printWarning('error', format, args);
}
}
}
export let isWritingAppendedStack = false;
function printWarning(level, format, args, currentStack) {
function printWarning(level, format, args) {
if (__DEV__) {
if (ReactSharedInternals.getCurrentStack) {
const stack = ReactSharedInternals.getCurrentStack(currentStack);
const stack = ReactSharedInternals.getCurrentStack();
if (stack !== '') {
isWritingAppendedStack = true;
format += '%s';

View File

@ -18,7 +18,7 @@ export function setSuppressWarning(newSuppressWarning) {
export function warn(format, ...args) {
if (__DEV__) {
if (!suppressWarning) {
printWarning('warn', format, args, new Error('react-stack-top-frame'));
printWarning('warn', format, args);
}
}
}
@ -26,19 +26,19 @@ export function warn(format, ...args) {
export function error(format, ...args) {
if (__DEV__) {
if (!suppressWarning) {
printWarning('error', format, args, new Error('react-stack-top-frame'));
printWarning('error', format, args);
}
}
}
function printWarning(level, format, args, currentStack) {
function printWarning(level, format, args) {
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(currentStack);
const stack = ReactSharedInternals.getCurrentStack();
if (stack !== '') {
format += '%s';
args.push(stack);