Skip to content

Commit 75b1014

Browse files
sebmarkbageAndyPengc12
authored andcommitted
Include server component names in the componentStack in DEV (facebook#28415)
I'm a bit ambivalent about this one because it's not the main strategy that I plan on pursuing. I plan on replacing most DEV-only specific stacks like `console.error` stacks with a new take on owner stacks and native stacks. The future owner stacks may or may not be exposed to error boundaries in DEV but if they are they'd be a new errorInfo property since they're owner based and not available in prod. The use case in `console.error` mostly goes away in the future so this PR is mainly for error boundaries. It doesn't hurt to have it in there while I'm working on the better stacks though. The `componentStack` property exposed to error boundaries is more like production behavior similar to `new Error().stack` (which even in DEV won't ever expose owner stacks because `console.createTask` doesn't affect these). I'm not sure it's worth adding server components in DEV (this PR) because then you have forked behavior between dev and prod. However, since even in the future there won't be any other place to get the *parent* stack, maybe this can be useful information even if it's only dev. We could expose a third property on errorInfo that's DEV only and parent stack but including server components. That doesn't seem worth it over just having the stack differ in dev and prod. I don't plan on adding line/column number to these particular stacks. A follow up could be to add this to Fizz prerender too but only in DEV.
1 parent 4e02501 commit 75b1014

File tree

6 files changed

+177
-0
lines changed

6 files changed

+177
-0
lines changed

packages/react-client/src/__tests__/ReactFlight-test.js

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@
1010

1111
'use strict';
1212

13+
function normalizeCodeLocInfo(str) {
14+
return (
15+
str &&
16+
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
17+
return '\n in ' + name + (/\d/.test(m) ? ' (at **)' : '');
18+
})
19+
);
20+
}
21+
1322
const heldValues = [];
1423
let finalizationCallback;
1524
function FinalizationRegistryMock(callback) {
@@ -69,6 +78,14 @@ describe('ReactFlight', () => {
6978
error,
7079
};
7180
}
81+
componentDidCatch(error, errorInfo) {
82+
expect(error).toBe(this.state.error);
83+
if (this.props.expectedStack !== undefined) {
84+
expect(normalizeCodeLocInfo(errorInfo.componentStack)).toBe(
85+
this.props.expectedStack,
86+
);
87+
}
88+
}
7289
componentDidMount() {
7390
expect(this.state.hasError).toBe(true);
7491
expect(this.state.error).toBeTruthy();
@@ -900,6 +917,91 @@ describe('ReactFlight', () => {
900917
});
901918
});
902919

920+
it('should include server components in error boundary stacks in dev', async () => {
921+
const ClientErrorBoundary = clientReference(ErrorBoundary);
922+
923+
function Throw({value}) {
924+
throw value;
925+
}
926+
927+
const expectedStack = __DEV__
928+
? // TODO: This should include Throw but it doesn't have a Fiber.
929+
'\n in div' + '\n in ErrorBoundary (at **)' + '\n in App'
930+
: '\n in div' + '\n in ErrorBoundary (at **)';
931+
932+
function App() {
933+
return (
934+
<ClientErrorBoundary
935+
expectedMessage="This is a real Error."
936+
expectedStack={expectedStack}>
937+
<div>
938+
<Throw value={new TypeError('This is a real Error.')} />
939+
</div>
940+
</ClientErrorBoundary>
941+
);
942+
}
943+
944+
const transport = ReactNoopFlightServer.render(<App />, {
945+
onError(x) {
946+
if (__DEV__) {
947+
return 'a dev digest';
948+
}
949+
if (x instanceof Error) {
950+
return `digest("${x.message}")`;
951+
} else if (Array.isArray(x)) {
952+
return `digest([])`;
953+
} else if (typeof x === 'object' && x !== null) {
954+
return `digest({})`;
955+
}
956+
return `digest(${String(x)})`;
957+
},
958+
});
959+
960+
await act(() => {
961+
startTransition(() => {
962+
ReactNoop.render(ReactNoopFlightClient.read(transport));
963+
});
964+
});
965+
});
966+
967+
it('should include server components in warning stacks', async () => {
968+
function Component() {
969+
// Trigger key warning
970+
return <div>{[<span />]}</div>;
971+
}
972+
const ClientComponent = clientReference(Component);
973+
974+
function Indirection({children}) {
975+
return children;
976+
}
977+
978+
function App() {
979+
return (
980+
<Indirection>
981+
<ClientComponent />
982+
</Indirection>
983+
);
984+
}
985+
986+
const transport = ReactNoopFlightServer.render(<App />);
987+
988+
await expect(async () => {
989+
await act(() => {
990+
startTransition(() => {
991+
ReactNoop.render(ReactNoopFlightClient.read(transport));
992+
});
993+
});
994+
}).toErrorDev(
995+
'Each child in a list should have a unique "key" prop.\n' +
996+
'\n' +
997+
'Check the render method of `Component`. See https://reactjs.org/link/warning-keys for more information.\n' +
998+
' in span (at **)\n' +
999+
' in Component (at **)\n' +
1000+
' in Indirection (at **)\n' +
1001+
' in App (at **)',
1002+
);
1003+
});
1004+
9031005
it('should trigger the inner most error boundary inside a Client Component', async () => {
9041006
function ServerComponent() {
9051007
throw new Error('This was thrown in the Server Component.');

packages/react-devtools-shared/src/__tests__/componentStacks-test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,41 @@ describe('component stack', () => {
8686
'\n in Example (at **)',
8787
);
8888
});
89+
90+
// @reactVersion >=18.3
91+
it('should log the current component stack with debug info from promises', () => {
92+
const Child = () => {
93+
console.error('Test error.');
94+
console.warn('Test warning.');
95+
return null;
96+
};
97+
const ChildPromise = Promise.resolve(<Child />);
98+
ChildPromise.status = 'fulfilled';
99+
ChildPromise.value = <Child />;
100+
ChildPromise._debugInfo = [
101+
{
102+
name: 'ServerComponent',
103+
env: 'Server',
104+
},
105+
];
106+
const Parent = () => ChildPromise;
107+
const Grandparent = () => <Parent />;
108+
109+
act(() => render(<Grandparent />));
110+
111+
expect(mockError).toHaveBeenCalledWith(
112+
'Test error.',
113+
'\n in Child (at **)' +
114+
'\n in ServerComponent (at **)' +
115+
'\n in Parent (at **)' +
116+
'\n in Grandparent (at **)',
117+
);
118+
expect(mockWarn).toHaveBeenCalledWith(
119+
'Test warning.',
120+
'\n in Child (at **)' +
121+
'\n in ServerComponent (at **)' +
122+
'\n in Parent (at **)' +
123+
'\n in Grandparent (at **)',
124+
);
125+
});
89126
});

packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ export function describeBuiltInComponentFrame(
5050
return '\n' + prefix + name;
5151
}
5252

53+
export function describeDebugInfoFrame(name: string, env: ?string): string {
54+
return describeBuiltInComponentFrame(
55+
name + (env ? ' (' + env + ')' : ''),
56+
null,
57+
);
58+
}
59+
5360
let reentry = false;
5461
let componentFrameCache;
5562
if (__DEV__) {

packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
describeBuiltInComponentFrame,
2020
describeFunctionComponentFrame,
2121
describeClassComponentFrame,
22+
describeDebugInfoFrame,
2223
} from './DevToolsComponentStackFrame';
2324

2425
export function describeFiber(
@@ -87,6 +88,16 @@ export function getStackByFiberInDevAndProd(
8788
let node: Fiber = workInProgress;
8889
do {
8990
info += describeFiber(workTagMap, node, currentDispatcherRef);
91+
// Add any Server Component stack frames in reverse order.
92+
const debugInfo = node._debugInfo;
93+
if (debugInfo) {
94+
for (let i = debugInfo.length - 1; i >= 0; i--) {
95+
const entry = debugInfo[i];
96+
if (typeof entry.name === 'string') {
97+
info += describeDebugInfoFrame(entry.name, entry.env);
98+
}
99+
}
100+
}
90101
// $FlowFixMe[incompatible-type] we bail out when we get a null
91102
node = node.return;
92103
} while (node);

packages/react-reconciler/src/ReactFiberComponentStack.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
describeBuiltInComponentFrame,
2727
describeFunctionComponentFrame,
2828
describeClassComponentFrame,
29+
describeDebugInfoFrame,
2930
} from 'shared/ReactComponentStackFrame';
3031

3132
function describeFiber(fiber: Fiber): string {
@@ -64,6 +65,18 @@ export function getStackByFiberInDevAndProd(workInProgress: Fiber): string {
6465
let node: Fiber = workInProgress;
6566
do {
6667
info += describeFiber(node);
68+
if (__DEV__) {
69+
// Add any Server Component stack frames in reverse order.
70+
const debugInfo = node._debugInfo;
71+
if (debugInfo) {
72+
for (let i = debugInfo.length - 1; i >= 0; i--) {
73+
const entry = debugInfo[i];
74+
if (typeof entry.name === 'string') {
75+
info += describeDebugInfoFrame(entry.name, entry.env);
76+
}
77+
}
78+
}
79+
}
6780
// $FlowFixMe[incompatible-type] we bail out when we get a null
6881
node = node.return;
6982
} while (node);

packages/shared/ReactComponentStackFrame.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ export function describeBuiltInComponentFrame(
5151
}
5252
}
5353

54+
export function describeDebugInfoFrame(name: string, env: ?string): string {
55+
return describeBuiltInComponentFrame(
56+
name + (env ? ' (' + env + ')' : ''),
57+
null,
58+
);
59+
}
60+
5461
let reentry = false;
5562
let componentFrameCache;
5663
if (__DEV__) {

0 commit comments

Comments
 (0)