Turn on key spread warning in jsx-runtime for everyone (#25697)
This improves the error message a bit and ensures that we recommend putting the key first, not last, which ensures that the faster `jsx-runtime` is used. This only affects the modern "automatic" JSX transform.
Sebastian Markbåge committed
Nov 16, 2022 at 18:57 UTC
07f46ecf2e559d03699263932b3ec82fb1099f1c
11 files changed
+149
-108
packages/react/src/ReactElementValidator.js
+109
-90
@@ -279,6 +279,8 @@ function validateFragmentProps(fragment) {
279
}
280
}
281
282
+const didWarnAboutKeySpread = {};
283
+
284
export function jsxWithValidation(
285
type,
286
props,
@@ -287,115 +289,132 @@ export function jsxWithValidation(
289
source,
290
self,
291
) {
290
- const validType = isValidElementType(type);
291
-
292
- // We warn in this case but don't throw. We expect the element creation to
293
- // succeed and there will likely be errors in render.
294
- if (!validType) {
295
- let info = '';
296
- if (
297
- type === undefined ||
298
- (typeof type === 'object' &&
299
- type !== null &&
300
- Object.keys(type).length === 0)
301
- ) {
302
- info +=
303
- ' You likely forgot to export your component from the file ' +
304
- "it's defined in, or you might have mixed up default and named imports.";
305
- }
292
+ if (__DEV__) {
293
+ const validType = isValidElementType(type);
294
+
295
+ // We warn in this case but don't throw. We expect the element creation to
296
+ // succeed and there will likely be errors in render.
297
+ if (!validType) {
298
+ let info = '';
299
+ if (
300
+ type === undefined ||
301
+ (typeof type === 'object' &&
302
+ type !== null &&
303
+ Object.keys(type).length === 0)
304
+ ) {
305
+ info +=
306
+ ' You likely forgot to export your component from the file ' +
307
+ "it's defined in, or you might have mixed up default and named imports.";
308
+ }
309
307
- const sourceInfo = getSourceInfoErrorAddendum(source);
308
- if (sourceInfo) {
309
- info += sourceInfo;
310
- } else {
311
- info += getDeclarationErrorAddendum();
312
- }
310
+ const sourceInfo = getSourceInfoErrorAddendum(source);
311
+ if (sourceInfo) {
312
+ info += sourceInfo;
313
+ } else {
314
+ info += getDeclarationErrorAddendum();
315
+ }
316
314
- let typeString;
315
- if (type === null) {
316
- typeString = 'null';
317
- } else if (isArray(type)) {
318
- typeString = 'array';
319
- } else if (type !== undefined && type.$$typeof === REACT_ELEMENT_TYPE) {
320
- typeString = `<${getComponentNameFromType(type.type) || 'Unknown'} />`;
321
- info =
322
- ' Did you accidentally export a JSX literal instead of a component?';
323
- } else {
324
- typeString = typeof type;
325
- }
317
+ let typeString;
318
+ if (type === null) {
319
+ typeString = 'null';
320
+ } else if (isArray(type)) {
321
+ typeString = 'array';
322
+ } else if (type !== undefined && type.$$typeof === REACT_ELEMENT_TYPE) {
323
+ typeString = `<${getComponentNameFromType(type.type) || 'Unknown'} />`;
324
+ info =
325
+ ' Did you accidentally export a JSX literal instead of a component?';
326
+ } else {
327
+ typeString = typeof type;
328
+ }
329
327
- if (__DEV__) {
328
- console.error(
329
- 'React.jsx: type is invalid -- expected a string (for ' +
330
- 'built-in components) or a class/function (for composite ' +
331
- 'components) but got: %s.%s',
332
- typeString,
333
- info,
334
- );
330
+ if (__DEV__) {
331
+ console.error(
332
+ 'React.jsx: type is invalid -- expected a string (for ' +
333
+ 'built-in components) or a class/function (for composite ' +
334
+ 'components) but got: %s.%s',
335
+ typeString,
336
+ info,
337
+ );
338
+ }
339
}
336
- }
340
338
- const element = jsxDEV(type, props, key, source, self);
339
-
340
- // The result can be nullish if a mock or a custom function is used.
341
- // TODO: Drop this when these are no longer allowed as the type argument.
342
- if (element == null) {
343
- return element;
344
- }
345
-
346
- // Skip key warning if the type isn't valid since our key validation logic
347
- // doesn't expect a non-string/function type and can throw confusing errors.
348
- // We don't want exception behavior to differ between dev and prod.
349
- // (Rendering will throw with a helpful message and as soon as the type is
350
- // fixed, the key warnings will appear.)
341
+ const element = jsxDEV(type, props, key, source, self);
342
352
- if (validType) {
353
- const children = props.children;
354
- if (children !== undefined) {
355
- if (isStaticChildren) {
356
- if (isArray(children)) {
357
- for (let i = 0; i < children.length; i++) {
358
- validateChildKeys(children[i], type);
359
- }
343
+ // The result can be nullish if a mock or a custom function is used.
344
+ // TODO: Drop this when these are no longer allowed as the type argument.
345
+ if (element == null) {
346
+ return element;
347
+ }
348
361
- if (Object.freeze) {
362
- Object.freeze(children);
349
+ // Skip key warning if the type isn't valid since our key validation logic
350
+ // doesn't expect a non-string/function type and can throw confusing errors.
351
+ // We don't want exception behavior to differ between dev and prod.
352
+ // (Rendering will throw with a helpful message and as soon as the type is
353
+ // fixed, the key warnings will appear.)
354
+
355
+ if (validType) {
356
+ const children = props.children;
357
+ if (children !== undefined) {
358
+ if (isStaticChildren) {
359
+ if (isArray(children)) {
360
+ for (let i = 0; i < children.length; i++) {
361
+ validateChildKeys(children[i], type);
362
+ }
363
+
364
+ if (Object.freeze) {
365
+ Object.freeze(children);
366
+ }
367
+ } else {
368
+ if (__DEV__) {
369
+ console.error(
370
+ 'React.jsx: Static children should always be an array. ' +
371
+ 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' +
372
+ 'Use the Babel transform instead.',
373
+ );
374
+ }
375
}
376
} else {
365
- if (__DEV__) {
366
- console.error(
367
- 'React.jsx: Static children should always be an array. ' +
368
- 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' +
369
- 'Use the Babel transform instead.',
370
- );
371
- }
377
+ validateChildKeys(children, type);
378
}
373
- } else {
374
- validateChildKeys(children, type);
379
}
380
}
377
- }
381
379
- if (__DEV__) {
382
if (warnAboutSpreadingKeyToJSX) {
383
if (hasOwnProperty.call(props, 'key')) {
382
- console.error(
383
- 'React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
384
- 'Explicitly pass a key after spreading props in your JSX call. ' +
385
- 'E.g. <%s {...props} key={key} />',
386
- getComponentNameFromType(type) || 'ComponentName',
387
- );
384
+ const componentName = getComponentNameFromType(type);
385
+ const keys = Object.keys(props).filter(k => k !== 'key');
386
+ const beforeExample =
387
+ keys.length > 0
388
+ ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}'
389
+ : '{key: someKey}';
390
+ if (!didWarnAboutKeySpread[componentName + beforeExample]) {
391
+ const afterExample =
392
+ keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}';
393
+ console.error(
394
+ 'A props object containing a "key" prop is being spread into JSX:\n' +
395
+ ' let props = %s;\n' +
396
+ ' <%s {...props} />\n' +
397
+ 'React keys must be passed directly to JSX without using spread:\n' +
398
+ ' let props = %s;\n' +
399
+ ' <%s key={someKey} {...props} />',
400
+ beforeExample,
401
+ componentName,
402
+ afterExample,
403
+ componentName,
404
+ );
405
+ didWarnAboutKeySpread[componentName + beforeExample] = true;
406
+ }
407
}
408
}
390
- }
409
392
- if (type === REACT_FRAGMENT_TYPE) {
393
- validateFragmentProps(element);
394
- } else {
395
- validatePropTypes(element);
396
- }
410
+ if (type === REACT_FRAGMENT_TYPE) {
411
+ validateFragmentProps(element);
412
+ } else {
413
+ validatePropTypes(element);
414
+ }
415
398
- return element;
416
+ return element;
417
+ }
418
}
419
420
// These two functions exist to still get child warnings in dev
packages/react/src/__tests__/ReactElementJSX-test.js
+7
-4
@@ -275,16 +275,19 @@ describe('ReactElement.jsx', () => {
275
class Parent extends React.Component {
276
render() {
277
return JSXRuntime.jsx('div', {
278
- children: [JSXRuntime.jsx(Child, {key: '0'})],
278
+ children: [JSXRuntime.jsx(Child, {key: '0', prop: 'hi'})],
279
});
280
}
281
}
282
expect(() =>
283
ReactDOM.render(JSXRuntime.jsx(Parent, {}), container),
284
).toErrorDev(
285
- 'Warning: React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
286
- 'Explicitly pass a key after spreading props in your JSX call. ' +
287
- 'E.g. <Child {...props} key={key} />',
285
+ 'Warning: A props object containing a "key" prop is being spread into JSX:\n' +
286
+ ' let props = {key: someKey, prop: ...};\n' +
287
+ ' <Child {...props} />\n' +
288
+ 'React keys must be passed directly to JSX without using spread:\n' +
289
+ ' let props = {prop: ...};\n' +
290
+ ' <Child key={someKey} {...props} />',
291
);
292
});
293
}
packages/react/src/jsx/ReactJSXElementValidator.js
+25
-6
@@ -294,6 +294,8 @@ function validateFragmentProps(fragment) {
294
}
295
}
296
297
+const didWarnAboutKeySpread = {};
298
+
299
export function jsxWithValidation(
300
type,
301
props,
@@ -390,12 +392,29 @@ export function jsxWithValidation(
392
393
if (warnAboutSpreadingKeyToJSX) {
394
if (hasOwnProperty.call(props, 'key')) {
393
- console.error(
394
- 'React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
395
- 'Explicitly pass a key after spreading props in your JSX call. ' +
396
- 'E.g. <%s {...props} key={key} />',
397
- getComponentNameFromType(type) || 'ComponentName',
398
- );
395
+ const componentName = getComponentNameFromType(type);
396
+ const keys = Object.keys(props).filter(k => k !== 'key');
397
+ const beforeExample =
398
+ keys.length > 0
399
+ ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}'
400
+ : '{key: someKey}';
401
+ if (!didWarnAboutKeySpread[componentName + beforeExample]) {
402
+ const afterExample =
403
+ keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}';
404
+ console.error(
405
+ 'A props object containing a "key" prop is being spread into JSX:\n' +
406
+ ' let props = %s;\n' +
407
+ ' <%s {...props} />\n' +
408
+ 'React keys must be passed directly to JSX without using spread:\n' +
409
+ ' let props = %s;\n' +
410
+ ' <%s key={someKey} {...props} />',
411
+ beforeExample,
412
+ componentName,
413
+ afterExample,
414
+ componentName,
415
+ );
416
+ didWarnAboutKeySpread[componentName + beforeExample] = true;
417
+ }
418
}
419
}
420
packages/shared/ReactFeatureFlags.js
+1
-1
@@ -217,7 +217,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate lat
217
218
// Enables a warning when trying to spread a 'key' to an element;
219
// a deprecated pattern we want to get rid of in the future
220
-export const warnAboutSpreadingKeyToJSX = false;
220
+export const warnAboutSpreadingKeyToJSX = true;
221
222
export const warnAboutStringRefs = false;
223
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
-1
@@ -48,7 +48,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
48
export const enableTrustedTypesIntegration = false;
49
export const disableTextareaChildren = false;
50
export const disableModulePatternComponents = false;
51
-export const warnAboutSpreadingKeyToJSX = false;
51
+export const warnAboutSpreadingKeyToJSX = true;
52
export const enableSuspenseAvoidThisFallback = false;
53
export const enableSuspenseAvoidThisFallbackFizz = false;
54
export const enableCPUSuspense = true;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = false;
40
export const disableModulePatternComponents = false;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableSuspenseAvoidThisFallback = false;
43
export const enableSuspenseAvoidThisFallbackFizz = false;
44
export const enableCPUSuspense = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = false;
40
export const disableModulePatternComponents = false;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableSuspenseAvoidThisFallback = false;
43
export const enableSuspenseAvoidThisFallbackFizz = false;
44
export const enableCPUSuspense = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = false;
40
export const disableModulePatternComponents = false;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableComponentStackLocations = false;
43
export const enableLegacyFBSupport = false;
44
export const enableFilterEmptyStringAttributesDOM = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = false;
40
export const disableModulePatternComponents = true;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableSuspenseAvoidThisFallback = true;
43
export const enableSuspenseAvoidThisFallbackFizz = false;
44
export const enableCPUSuspense = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = false;
40
export const disableModulePatternComponents = false;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableSuspenseAvoidThisFallback = false;
43
export const enableSuspenseAvoidThisFallbackFizz = false;
44
export const enableCPUSuspense = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
-1
@@ -38,7 +38,7 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
38
export const enableTrustedTypesIntegration = false;
39
export const disableTextareaChildren = __EXPERIMENTAL__;
40
export const disableModulePatternComponents = true;
41
-export const warnAboutSpreadingKeyToJSX = false;
41
+export const warnAboutSpreadingKeyToJSX = true;
42
export const enableSuspenseAvoidThisFallback = true;
43
export const enableSuspenseAvoidThisFallbackFizz = false;
44
export const enableCPUSuspense = true;