Skip to content

Commit a7f5872

Browse files
sebmarkbageAndyPengc12
authored andcommitted
Use getComponentNameFromType for debug info for the key warning (facebook#27930)
If this is a client reference we shouldn't dot into it, which would throw in the proxy. Interestingly our client references don't really have a `name` associated with them for debug information so a component type doesn't show up in error logs even though it seems like it should.
1 parent d007617 commit a7f5872

File tree

5 files changed

+55
-16
lines changed

5 files changed

+55
-16
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,22 @@ describe('ReactFlight', () => {
10091009
ReactNoopFlightClient.read(transport);
10101010
});
10111011

1012+
it('should warn in DEV a child is missing keys', () => {
1013+
function ParentClient({children}) {
1014+
return children;
1015+
}
1016+
const Parent = clientReference(ParentClient);
1017+
expect(() => {
1018+
const transport = ReactNoopFlightServer.render(
1019+
<Parent>{Array(6).fill(<div>no key</div>)}</Parent>,
1020+
);
1021+
ReactNoopFlightClient.read(transport);
1022+
}).toErrorDev(
1023+
'Each child in a list should have a unique "key" prop. ' +
1024+
'See https://reactjs.org/link/warning-keys for more information.',
1025+
);
1026+
});
1027+
10121028
it('should error if a class instance is passed to a host component', () => {
10131029
class Foo {
10141030
method() {}

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,29 @@ describe('ReactFlightDOMBrowser', () => {
590590
expect(reportedErrors).toEqual(['for reasons']);
591591
});
592592

593+
it('should warn in DEV a child is missing keys', async () => {
594+
function ParentClient({children}) {
595+
return children;
596+
}
597+
const Parent = clientExports(ParentClient);
598+
const ParentModule = clientExports({Parent: ParentClient});
599+
await expect(async () => {
600+
const stream = ReactServerDOMServer.renderToReadableStream(
601+
<>
602+
<Parent>{Array(6).fill(<div>no key</div>)}</Parent>
603+
<ParentModule.Parent>
604+
{Array(6).fill(<div>no key</div>)}
605+
</ParentModule.Parent>
606+
</>,
607+
webpackMap,
608+
);
609+
await ReactServerDOMClient.createFromReadableStream(stream);
610+
}).toErrorDev(
611+
'Each child in a list should have a unique "key" prop. ' +
612+
'See https://reactjs.org/link/warning-keys for more information.',
613+
);
614+
});
615+
593616
it('basic use(promise)', async () => {
594617
function Server() {
595618
return (

packages/react/src/ReactElementValidator.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,7 @@ function getCurrentComponentErrorInfo(parentType) {
9696
let info = getDeclarationErrorAddendum();
9797

9898
if (!info) {
99-
const parentName =
100-
typeof parentType === 'string'
101-
? parentType
102-
: parentType.displayName || parentType.name;
99+
const parentName = getComponentNameFromType(parentType);
103100
if (parentName) {
104101
info = `\n\nCheck the top-level render call using <${parentName}>.`;
105102
}

packages/react/src/jsx/ReactJSXElementValidator.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ function getCurrentComponentErrorInfo(parentType) {
108108
let info = getDeclarationErrorAddendum();
109109

110110
if (!info) {
111-
const parentName =
112-
typeof parentType === 'string'
113-
? parentType
114-
: parentType.displayName || parentType.name;
111+
const parentName = getComponentNameFromType(parentType);
115112
if (parentName) {
116113
info = `\n\nCheck the top-level render call using <${parentName}>.`;
117114
}

packages/shared/getComponentNameFromType.js

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,21 +52,19 @@ function getContextName(type: ReactContext<any>) {
5252
return type.displayName || 'Context';
5353
}
5454

55+
const REACT_CLIENT_REFERENCE = Symbol.for('react.client.reference');
56+
5557
// Note that the reconciler package should generally prefer to use getComponentNameFromFiber() instead.
5658
export default function getComponentNameFromType(type: mixed): string | null {
5759
if (type == null) {
5860
// Host root, text node or just invalid type.
5961
return null;
6062
}
61-
if (__DEV__) {
62-
if (typeof (type: any).tag === 'number') {
63-
console.error(
64-
'Received an unexpected object in getComponentNameFromType(). ' +
65-
'This is likely a bug in React. Please file an issue.',
66-
);
67-
}
68-
}
6963
if (typeof type === 'function') {
64+
if ((type: any).$$typeof === REACT_CLIENT_REFERENCE) {
65+
// TODO: Create a convention for naming client references with debug info.
66+
return null;
67+
}
7068
return (type: any).displayName || type.name || null;
7169
}
7270
if (typeof type === 'string') {
@@ -96,6 +94,14 @@ export default function getComponentNameFromType(type: mixed): string | null {
9694
}
9795
}
9896
if (typeof type === 'object') {
97+
if (__DEV__) {
98+
if (typeof (type: any).tag === 'number') {
99+
console.error(
100+
'Received an unexpected object in getComponentNameFromType(). ' +
101+
'This is likely a bug in React. Please file an issue.',
102+
);
103+
}
104+
}
99105
switch (type.$$typeof) {
100106
case REACT_CONTEXT_TYPE:
101107
const context: ReactContext<any> = (type: any);

0 commit comments

Comments
 (0)