Skip to content

Commit

Permalink
Implement key warning in Fizz
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage committed May 16, 2024
1 parent e7d5c0f commit 3bc8280
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 2 deletions.
10 changes: 9 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1833,10 +1833,18 @@ describe('ReactDOMFizzServer', () => {
expect(mockError).toHaveBeenCalledWith(
'Warning: Each child in a list should have a unique "key" prop.%s%s' +
' See https://react.dev/link/warning-keys for more information.%s',
'\n\nCheck the top-level render call using <div>.',
gate(flags => flags.enableOwnerStacks)
? // We currently don't track owners in Fizz which is responsible for this frame.
''
: '\n\nCheck the top-level render call using <div>.',
'',
'\n' +
' in span (at **)\n' +
// TODO: Because this validates after the div has been mounted, it is part of
// the parent stack but since owner stacks will switch to owners this goes away again.
(gate(flags => flags.enableOwnerStacks)
? ' in div (at **)\n'
: '') +
' in B (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ describe('ReactDOMServer', () => {
}

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

function App() {
Expand Down
75 changes: 75 additions & 0 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ export opaque type Request = {
onPostpone: (reason: string, postponeInfo: ThrownInfo) => void,
// Form state that was the result of an MPA submission, if it was provided.
formState: null | ReactFormState<any, any>,
// DEV-only, warning dedupe
didWarnForKey?: null | WeakSet<ComponentStackNode>,
};

// This is a default heuristic for how to split up the HTML content into progressive
Expand Down Expand Up @@ -409,6 +411,9 @@ export function createRequest(
onFatalError: onFatalError === undefined ? noop : onFatalError,
formState: formState === undefined ? null : formState,
};
if (__DEV__) {
request.didWarnForKey = null;
}
// This segment represents the root fallback.
const rootSegment = createPendingSegment(
request,
Expand Down Expand Up @@ -787,6 +792,19 @@ function createClassComponentStack(
};
}
function createComponentStackFromType(
task: Task,
type: Function | string,
): ComponentStackNode {
if (typeof type === 'string') {
return createBuiltInComponentStack(task, type);
}
if (shouldConstruct(type)) {
return createClassComponentStack(task, type);
}
return createFunctionComponentStack(task, type);
}
type ThrownInfo = {
componentStack?: string,
};
Expand Down Expand Up @@ -2597,6 +2615,59 @@ function replayFragment(
}
}

function warnForMissingKey(request: Request, task: Task, child: mixed): void {
if (__DEV__) {
if (
child === null ||
typeof child !== 'object' ||
(child.$$typeof !== REACT_ELEMENT_TYPE &&
child.$$typeof !== REACT_PORTAL_TYPE)
) {
return;
}

if (
!child._store ||
((child._store.validated || child.key != null) &&
child._store.validated !== 2)
) {
return;
}

if (typeof child._store !== 'object') {
throw new Error(
'React Component in warnForMissingKey should have a _store. ' +
'This error is likely caused by a bug in React. Please file an issue.',
);
}

// $FlowFixMe[cannot-write] unable to narrow type from mixed to writable object
child._store.validated = 1;

let didWarnForKey = request.didWarnForKey;
if (didWarnForKey == null) {
didWarnForKey = request.didWarnForKey = new WeakSet();
}
const parentStackFrame = task.componentStack;
if (parentStackFrame === null || didWarnForKey.has(parentStackFrame)) {
// We already warned for other children in this parent.
return;
}
didWarnForKey.add(parentStackFrame);

// We create a fake component stack for the child to log the stack trace from.
const stackFrame = createComponentStackFromType(task, (child: any).type);
task.componentStack = stackFrame;
console.error(
'Each child in a list should have a unique "key" prop.' +
'%s%s See https://react.dev/link/warning-keys for more information.',
'',
'',
);
task.componentStack = stackFrame.parent;
}
}

function renderChildrenArray(
request: Request,
task: Task,
Expand All @@ -2618,6 +2689,7 @@ function renderChildrenArray(
return;
}
}

const prevTreeContext = task.treeContext;
const totalChildren = children.length;

Expand Down Expand Up @@ -2650,6 +2722,9 @@ function renderChildrenArray(

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

0 comments on commit 3bc8280

Please sign in to comment.