Skip to content

Commit c62d157

Browse files
committed
Implement key warning in Fizz
This is just additional for running warnings in the server logs in addition to the ones that happen during hydration but it's good to have both for parity and being able to fail SSR tests early. This also demonstrates per request warning deduping so that they don't just happen once when the server remains alive.
1 parent 1bbb3e5 commit c62d157

File tree

3 files changed

+85
-2
lines changed

3 files changed

+85
-2
lines changed

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1833,10 +1833,18 @@ describe('ReactDOMFizzServer', () => {
18331833
expect(mockError).toHaveBeenCalledWith(
18341834
'Warning: Each child in a list should have a unique "key" prop.%s%s' +
18351835
' See https://react.dev/link/warning-keys for more information.%s',
1836-
'\n\nCheck the top-level render call using <div>.',
1836+
gate(flags => flags.enableOwnerStacks)
1837+
? // We currently don't track owners in Fizz which is responsible for this frame.
1838+
''
1839+
: '\n\nCheck the top-level render call using <div>.',
18371840
'',
18381841
'\n' +
18391842
' in span (at **)\n' +
1843+
// TODO: Because this validates after the div has been mounted, it is part of
1844+
// the parent stack but since owner stacks will switch to owners this goes away again.
1845+
(gate(flags => flags.enableOwnerStacks)
1846+
? ' in div (at **)\n'
1847+
: '') +
18401848
' in B (at **)\n' +
18411849
' in Suspense (at **)\n' +
18421850
' in div (at **)\n' +

packages/react-dom/src/__tests__/ReactServerRendering-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,7 @@ describe('ReactDOMServer', () => {
819819
}
820820

821821
function Child() {
822-
return [<A key="1" />, <B key="2" />, <span ariaTypo2="no" />];
822+
return [<A key="1" />, <B key="2" />, <span ariaTypo2="no" key="3" />];
823823
}
824824

825825
function App() {

packages/react-server/src/ReactFizzServer.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,8 @@ export opaque type Request = {
337337
onPostpone: (reason: string, postponeInfo: ThrownInfo) => void,
338338
// Form state that was the result of an MPA submission, if it was provided.
339339
formState: null | ReactFormState<any, any>,
340+
// DEV-only, warning dedupe
341+
didWarnForKey?: null | WeakSet<ComponentStackNode>,
340342
};
341343

342344
// This is a default heuristic for how to split up the HTML content into progressive
@@ -409,6 +411,9 @@ export function createRequest(
409411
onFatalError: onFatalError === undefined ? noop : onFatalError,
410412
formState: formState === undefined ? null : formState,
411413
};
414+
if (__DEV__) {
415+
request.didWarnForKey = null;
416+
}
412417
// This segment represents the root fallback.
413418
const rootSegment = createPendingSegment(
414419
request,
@@ -787,6 +792,19 @@ function createClassComponentStack(
787792
};
788793
}
789794

795+
function createComponentStackFromType(
796+
task: Task,
797+
type: Function | string,
798+
): ComponentStackNode {
799+
if (typeof type === 'string') {
800+
return createBuiltInComponentStack(task, type);
801+
}
802+
if (shouldConstruct(type)) {
803+
return createClassComponentStack(task, type);
804+
}
805+
return createFunctionComponentStack(task, type);
806+
}
807+
790808
type ThrownInfo = {
791809
componentStack?: string,
792810
};
@@ -2597,6 +2615,59 @@ function replayFragment(
25972615
}
25982616
}
25992617

2618+
function warnForMissingKey(request: Request, task: Task, child: mixed): void {
2619+
if (__DEV__) {
2620+
if (
2621+
child === null ||
2622+
typeof child !== 'object' ||
2623+
(child.$$typeof !== REACT_ELEMENT_TYPE &&
2624+
child.$$typeof !== REACT_PORTAL_TYPE)
2625+
) {
2626+
return;
2627+
}
2628+
2629+
if (
2630+
!child._store ||
2631+
((child._store.validated || child.key != null) &&
2632+
child._store.validated !== 2)
2633+
) {
2634+
return;
2635+
}
2636+
2637+
if (typeof child._store !== 'object') {
2638+
throw new Error(
2639+
'React Component in warnForMissingKey should have a _store. ' +
2640+
'This error is likely caused by a bug in React. Please file an issue.',
2641+
);
2642+
}
2643+
2644+
// $FlowFixMe[cannot-write] unable to narrow type from mixed to writable object
2645+
child._store.validated = 1;
2646+
2647+
let didWarnForKey = request.didWarnForKey;
2648+
if (didWarnForKey == null) {
2649+
didWarnForKey = request.didWarnForKey = new WeakSet();
2650+
}
2651+
const parentStackFrame = task.componentStack;
2652+
if (parentStackFrame === null || didWarnForKey.has(parentStackFrame)) {
2653+
// We already warned for other children in this parent.
2654+
return;
2655+
}
2656+
didWarnForKey.add(parentStackFrame);
2657+
2658+
// We create a fake component stack for the child to log the stack trace from.
2659+
const stackFrame = createComponentStackFromType(task, (child: any).type);
2660+
task.componentStack = stackFrame;
2661+
console.error(
2662+
'Each child in a list should have a unique "key" prop.' +
2663+
'%s%s See https://react.dev/link/warning-keys for more information.',
2664+
'',
2665+
'',
2666+
);
2667+
task.componentStack = stackFrame.parent;
2668+
}
2669+
}
2670+
26002671
function renderChildrenArray(
26012672
request: Request,
26022673
task: Task,
@@ -2618,6 +2689,7 @@ function renderChildrenArray(
26182689
return;
26192690
}
26202691
}
2692+
26212693
const prevTreeContext = task.treeContext;
26222694
const totalChildren = children.length;
26232695

@@ -2650,6 +2722,9 @@ function renderChildrenArray(
26502722

26512723
for (let i = 0; i < totalChildren; i++) {
26522724
const node = children[i];
2725+
if (__DEV__) {
2726+
warnForMissingKey(request, task, node);
2727+
}
26532728
task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i);
26542729
// We need to use the non-destructive form so that we can safely pop back
26552730
// up and render the sibling if something suspends.

0 commit comments

Comments
 (0)