Skip to content

Commit ba6a9e9

Browse files
authored
[Flight] Warn for keyless fragments in an array (facebook#30588)
Conceptually this is the same as rendering this as if it was a built-in Server Component.
1 parent 8a70d31 commit ba6a9e9

File tree

2 files changed

+41
-18
lines changed

2 files changed

+41
-18
lines changed

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

+19
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,25 @@ describe('ReactFlight', () => {
16321632
}).toErrorDev('Each child in a list should have a unique "key" prop.');
16331633
});
16341634

1635+
// @gate !__DEV__ || enableOwnerStacks
1636+
it('should warn in DEV a child is missing keys on a fragment', () => {
1637+
expect(() => {
1638+
// While we're on the server we need to have the Server version active to track component stacks.
1639+
jest.resetModules();
1640+
jest.mock('react', () => ReactServer);
1641+
const transport = ReactNoopFlightServer.render(
1642+
ReactServer.createElement(
1643+
'div',
1644+
null,
1645+
Array(6).fill(ReactServer.createElement(ReactServer.Fragment)),
1646+
),
1647+
);
1648+
jest.resetModules();
1649+
jest.mock('react', () => React);
1650+
ReactNoopFlightClient.read(transport);
1651+
}).toErrorDev('Each child in a list should have a unique "key" prop.');
1652+
});
1653+
16351654
it('should warn in DEV a child is missing keys in client component', async () => {
16361655
function ParentClient({children}) {
16371656
return children;

packages/react-server/src/ReactFlightServer.js

+22-18
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,7 @@ function renderFunctionComponent<Props>(
10291029
const componentDebugID = debugID;
10301030
const componentName =
10311031
(Component: any).displayName || Component.name || '';
1032-
const componentEnv = request.environmentName();
1032+
const componentEnv = (0, request.environmentName)();
10331033
request.pendingChunks++;
10341034
componentDebugInfo = ({
10351035
name: componentName,
@@ -1056,14 +1056,8 @@ function renderFunctionComponent<Props>(
10561056
// We've emitted the latest environment for this task so we track that.
10571057
task.environmentName = componentEnv;
10581058

1059-
if (enableOwnerStacks) {
1060-
warnForMissingKey(
1061-
request,
1062-
key,
1063-
validated,
1064-
componentDebugInfo,
1065-
task.debugTask,
1066-
);
1059+
if (enableOwnerStacks && validated === 2) {
1060+
warnForMissingKey(request, key, componentDebugInfo, task.debugTask);
10671061
}
10681062
}
10691063
prepareToUseHooksForComponent(prevThenableState, componentDebugInfo);
@@ -1256,15 +1250,10 @@ function renderFunctionComponent<Props>(
12561250
function warnForMissingKey(
12571251
request: Request,
12581252
key: null | string,
1259-
validated: number,
12601253
componentDebugInfo: ReactComponentInfo,
12611254
debugTask: null | ConsoleTask,
12621255
): void {
12631256
if (__DEV__) {
1264-
if (validated !== 2) {
1265-
return;
1266-
}
1267-
12681257
let didWarnForKey = request.didWarnForKey;
12691258
if (didWarnForKey == null) {
12701259
didWarnForKey = request.didWarnForKey = new WeakSet();
@@ -1573,6 +1562,21 @@ function renderElement(
15731562
} else if (type === REACT_FRAGMENT_TYPE && key === null) {
15741563
// For key-less fragments, we add a small optimization to avoid serializing
15751564
// it as a wrapper.
1565+
if (__DEV__ && enableOwnerStacks && validated === 2) {
1566+
// Create a fake owner node for the error stack.
1567+
const componentDebugInfo: ReactComponentInfo = {
1568+
name: 'Fragment',
1569+
env: (0, request.environmentName)(),
1570+
owner: task.debugOwner,
1571+
stack:
1572+
task.debugStack === null
1573+
? null
1574+
: filterStackTrace(request, task.debugStack, 1),
1575+
debugStack: task.debugStack,
1576+
debugTask: task.debugTask,
1577+
};
1578+
warnForMissingKey(request, key, componentDebugInfo, task.debugTask);
1579+
}
15761580
const prevImplicitSlot = task.implicitSlot;
15771581
if (task.keyPath === null) {
15781582
task.implicitSlot = true;
@@ -2921,7 +2925,7 @@ function emitErrorChunk(
29212925
if (__DEV__) {
29222926
let message;
29232927
let stack: ReactStackTrace;
2924-
let env = request.environmentName();
2928+
let env = (0, request.environmentName)();
29252929
try {
29262930
if (error instanceof Error) {
29272931
// eslint-disable-next-line react-internal/safe-string-coercion
@@ -3442,7 +3446,7 @@ function emitConsoleChunk(
34423446
}
34433447

34443448
// TODO: Don't double badge if this log came from another Flight Client.
3445-
const env = request.environmentName();
3449+
const env = (0, request.environmentName)();
34463450
const payload = [methodName, stackTrace, owner, env];
34473451
// $FlowFixMe[method-unbinding]
34483452
payload.push.apply(payload, args);
@@ -3611,7 +3615,7 @@ function retryTask(request: Request, task: Task): void {
36113615
request.writtenObjects.set(resolvedModel, serializeByValueID(task.id));
36123616

36133617
if (__DEV__) {
3614-
const currentEnv = request.environmentName();
3618+
const currentEnv = (0, request.environmentName)();
36153619
if (currentEnv !== task.environmentName) {
36163620
// The environment changed since we last emitted any debug information for this
36173621
// task. We emit an entry that just includes the environment name change.
@@ -3629,7 +3633,7 @@ function retryTask(request: Request, task: Task): void {
36293633
const json: string = stringify(resolvedModel);
36303634

36313635
if (__DEV__) {
3632-
const currentEnv = request.environmentName();
3636+
const currentEnv = (0, request.environmentName)();
36333637
if (currentEnv !== task.environmentName) {
36343638
// The environment changed since we last emitted any debug information for this
36353639
// task. We emit an entry that just includes the environment name change.

0 commit comments

Comments
 (0)