Skip to content

Commit 433068e

Browse files
authored
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.
1 parent af28f48 commit 433068e

8 files changed

+25
-39
lines changed

packages/react-reconciler/src/ReactCurrentFiber.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export function getCurrentFiberOwnerNameInDevOrNull(): string | null {
3333
return null;
3434
}
3535

36-
function getCurrentFiberStackInDev(stack: null | Error): string {
36+
function getCurrentFiberStackInDev(): string {
3737
if (__DEV__) {
3838
if (current === null) {
3939
return '';
@@ -43,7 +43,7 @@ function getCurrentFiberStackInDev(stack: null | Error): string {
4343
// TODO: The above comment is not actually true. We might be
4444
// in a commit phase or preemptive set state callback.
4545
if (enableOwnerStacks) {
46-
return getOwnerStackByFiberInDev(current, stack);
46+
return getOwnerStackByFiberInDev(current);
4747
}
4848
return getStackByFiberInDevAndProd(current);
4949
}

packages/react-reconciler/src/ReactFiberComponentStack.js

+1-15
Original file line numberDiff line numberDiff line change
@@ -91,27 +91,13 @@ function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
9191
return name ? describeBuiltInComponentFrame(name) : '';
9292
}
9393

94-
export function getOwnerStackByFiberInDev(
95-
workInProgress: Fiber,
96-
topStack: null | Error,
97-
): string {
94+
export function getOwnerStackByFiberInDev(workInProgress: Fiber): string {
9895
if (!enableOwnerStacks || !__DEV__) {
9996
return '';
10097
}
10198
try {
10299
let info = '';
103100

104-
if (topStack) {
105-
// Prefix with a filtered version of the currently executing
106-
// stack. This information will be available in the native
107-
// stack regardless but it's hidden since we're reprinting
108-
// the stack on top of it.
109-
const formattedTopStack = formatOwnerStack(topStack);
110-
if (formattedTopStack !== '') {
111-
info += '\n' + formattedTopStack;
112-
}
113-
}
114-
115101
if (workInProgress.tag === HostText) {
116102
// Text nodes never have an owner/stack because they're not created through JSX.
117103
// We use the parent since text nodes are always created through a host parent.

packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,18 @@ describe('ReactLazy', () => {
213213
unstable_isConcurrent: true,
214214
});
215215

216+
function App() {
217+
return (
218+
<Suspense fallback={<Text text="Loading..." />}>
219+
<LazyText text="Hi" />
220+
</Suspense>
221+
);
222+
}
223+
216224
let error;
217225
try {
218226
await act(() => {
219-
root.update(
220-
<Suspense fallback={<Text text="Loading..." />}>
221-
<LazyText text="Hi" />
222-
</Suspense>,
223-
);
227+
root.update(<App />);
224228
});
225229
} catch (e) {
226230
error = e;

packages/react/src/ReactOwnerStack.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ export function captureOwnerStack(): null | string {
2020
}
2121
// The current stack will be the owner stack if enableOwnerStacks is true
2222
// which it is always here. Otherwise it's the parent stack.
23-
return getCurrentStack(null);
23+
return getCurrentStack();
2424
}

packages/react/src/ReactSharedInternalsClient.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export type SharedStateClient = {
3535
thrownErrors: Array<mixed>,
3636

3737
// ReactDebugCurrentFrame
38-
getCurrentStack: null | ((stack: null | Error) => string),
38+
getCurrentStack: null | (() => string),
3939
};
4040

4141
export type RendererTask = boolean => RendererTask | null;
@@ -54,9 +54,7 @@ if (__DEV__) {
5454
ReactSharedInternals.didUsePromise = false;
5555
ReactSharedInternals.thrownErrors = [];
5656
// Stack implementation injected by the current renderer.
57-
ReactSharedInternals.getCurrentStack = (null:
58-
| null
59-
| ((stack: null | Error) => string));
57+
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
6058
}
6159

6260
export default ReactSharedInternals;

packages/react/src/ReactSharedInternalsServer.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export type SharedStateServer = {
3838
// DEV-only
3939

4040
// ReactDebugCurrentFrame
41-
getCurrentStack: null | ((stack: null | Error) => string),
41+
getCurrentStack: null | (() => string),
4242
};
4343

4444
export type RendererTask = boolean => RendererTask | null;
@@ -58,9 +58,7 @@ if (enableTaint) {
5858

5959
if (__DEV__) {
6060
// Stack implementation injected by the current renderer.
61-
ReactSharedInternals.getCurrentStack = (null:
62-
| null
63-
| ((stack: null | Error) => string));
61+
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
6462
}
6563

6664
export default ReactSharedInternals;

packages/shared/forks/consoleWithStackDev.rn.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,25 @@ export function setSuppressWarning(newSuppressWarning) {
2323
export function warn(format, ...args) {
2424
if (__DEV__) {
2525
if (!suppressWarning) {
26-
printWarning('warn', format, args, new Error('react-stack-top-frame'));
26+
printWarning('warn', format, args);
2727
}
2828
}
2929
}
3030

3131
export function error(format, ...args) {
3232
if (__DEV__) {
3333
if (!suppressWarning) {
34-
printWarning('error', format, args, new Error('react-stack-top-frame'));
34+
printWarning('error', format, args);
3535
}
3636
}
3737
}
3838

3939
export let isWritingAppendedStack = false;
4040

41-
function printWarning(level, format, args, currentStack) {
41+
function printWarning(level, format, args) {
4242
if (__DEV__) {
4343
if (ReactSharedInternals.getCurrentStack) {
44-
const stack = ReactSharedInternals.getCurrentStack(currentStack);
44+
const stack = ReactSharedInternals.getCurrentStack();
4545
if (stack !== '') {
4646
isWritingAppendedStack = true;
4747
format += '%s';

packages/shared/forks/consoleWithStackDev.www.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,27 @@ export function setSuppressWarning(newSuppressWarning) {
1818
export function warn(format, ...args) {
1919
if (__DEV__) {
2020
if (!suppressWarning) {
21-
printWarning('warn', format, args, new Error('react-stack-top-frame'));
21+
printWarning('warn', format, args);
2222
}
2323
}
2424
}
2525

2626
export function error(format, ...args) {
2727
if (__DEV__) {
2828
if (!suppressWarning) {
29-
printWarning('error', format, args, new Error('react-stack-top-frame'));
29+
printWarning('error', format, args);
3030
}
3131
}
3232
}
3333

34-
function printWarning(level, format, args, currentStack) {
34+
function printWarning(level, format, args) {
3535
if (__DEV__) {
3636
const React = require('react');
3737
const ReactSharedInternals =
3838
React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
3939
// Defensive in case this is fired before React is initialized.
4040
if (ReactSharedInternals != null && ReactSharedInternals.getCurrentStack) {
41-
const stack = ReactSharedInternals.getCurrentStack(currentStack);
41+
const stack = ReactSharedInternals.getCurrentStack();
4242
if (stack !== '') {
4343
format += '%s';
4444
args.push(stack);

0 commit comments

Comments
 (0)