From 64cad047e81f7bc945adb8c5d82953e9b6d097ad Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 30 Jan 2023 11:20:04 -0500 Subject: [PATCH 1/2] [cleanup] fully roll out warnAboutSpreadingKeyToJSX --- packages/react/src/ReactElementValidator.js | 61 +++++++++---------- .../src/__tests__/ReactElementJSX-test.js | 48 +++++++-------- .../react/src/jsx/ReactJSXElementValidator.js | 49 +++++++-------- packages/shared/ReactFeatureFlags.js | 1 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.native.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.testing.js | 1 - .../forks/ReactFeatureFlags.testing.www.js | 1 - .../forks/ReactFeatureFlags.www-dynamic.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - 13 files changed, 74 insertions(+), 94 deletions(-) diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index b67f52fe9f785..1cc7bcf475990 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -21,7 +21,6 @@ import { REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE, } from 'shared/ReactSymbols'; -import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags'; import checkPropTypes from 'shared/checkPropTypes'; import isArray from 'shared/isArray'; @@ -365,13 +364,11 @@ export function jsxWithValidation( Object.freeze(children); } } else { - if (__DEV__) { - console.error( - 'React.jsx: Static children should always be an array. ' + - 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' + - 'Use the Babel transform instead.', - ); - } + console.error( + 'React.jsx: Static children should always be an array. ' + + 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' + + 'Use the Babel transform instead.', + ); } } else { validateChildKeys(children, type); @@ -379,31 +376,29 @@ export function jsxWithValidation( } } - if (warnAboutSpreadingKeyToJSX) { - if (hasOwnProperty.call(props, 'key')) { - const componentName = getComponentNameFromType(type); - const keys = Object.keys(props).filter(k => k !== 'key'); - const beforeExample = - keys.length > 0 - ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' - : '{key: someKey}'; - if (!didWarnAboutKeySpread[componentName + beforeExample]) { - const afterExample = - keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; - console.error( - 'A props object containing a "key" prop is being spread into JSX:\n' + - ' let props = %s;\n' + - ' <%s {...props} />\n' + - 'React keys must be passed directly to JSX without using spread:\n' + - ' let props = %s;\n' + - ' <%s key={someKey} {...props} />', - beforeExample, - componentName, - afterExample, - componentName, - ); - didWarnAboutKeySpread[componentName + beforeExample] = true; - } + if (hasOwnProperty.call(props, 'key')) { + const componentName = getComponentNameFromType(type); + const keys = Object.keys(props).filter(k => k !== 'key'); + const beforeExample = + keys.length > 0 + ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' + : '{key: someKey}'; + if (!didWarnAboutKeySpread[componentName + beforeExample]) { + const afterExample = + keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; + console.error( + 'A props object containing a "key" prop is being spread into JSX:\n' + + ' let props = %s;\n' + + ' <%s {...props} />\n' + + 'React keys must be passed directly to JSX without using spread:\n' + + ' let props = %s;\n' + + ' <%s key={someKey} {...props} />', + beforeExample, + componentName, + afterExample, + componentName, + ); + didWarnAboutKeySpread[componentName + beforeExample] = true; } } diff --git a/packages/react/src/__tests__/ReactElementJSX-test.js b/packages/react/src/__tests__/ReactElementJSX-test.js index ad42046087c4d..4b7e70a073366 100644 --- a/packages/react/src/__tests__/ReactElementJSX-test.js +++ b/packages/react/src/__tests__/ReactElementJSX-test.js @@ -264,33 +264,31 @@ describe('ReactElement.jsx', () => { ); }); - if (require('shared/ReactFeatureFlags').warnAboutSpreadingKeyToJSX) { - it('should warn when keys are passed as part of props', () => { - const container = document.createElement('div'); - class Child extends React.Component { - render() { - return JSXRuntime.jsx('div', {}); - } + it('should warn when keys are passed as part of props', () => { + const container = document.createElement('div'); + class Child extends React.Component { + render() { + return JSXRuntime.jsx('div', {}); } - class Parent extends React.Component { - render() { - return JSXRuntime.jsx('div', { - children: [JSXRuntime.jsx(Child, {key: '0', prop: 'hi'})], - }); - } + } + class Parent extends React.Component { + render() { + return JSXRuntime.jsx('div', { + children: [JSXRuntime.jsx(Child, {key: '0', prop: 'hi'})], + }); } - expect(() => - ReactDOM.render(JSXRuntime.jsx(Parent, {}), container), - ).toErrorDev( - 'Warning: A props object containing a "key" prop is being spread into JSX:\n' + - ' let props = {key: someKey, prop: ...};\n' + - ' \n' + - 'React keys must be passed directly to JSX without using spread:\n' + - ' let props = {prop: ...};\n' + - ' ', - ); - }); - } + } + expect(() => + ReactDOM.render(JSXRuntime.jsx(Parent, {}), container), + ).toErrorDev( + 'Warning: A props object containing a "key" prop is being spread into JSX:\n' + + ' let props = {key: someKey, prop: ...};\n' + + ' \n' + + 'React keys must be passed directly to JSX without using spread:\n' + + ' let props = {prop: ...};\n' + + ' ', + ); + }); it('should not warn when unkeyed children are passed to jsxs', () => { const container = document.createElement('div'); diff --git a/packages/react/src/jsx/ReactJSXElementValidator.js b/packages/react/src/jsx/ReactJSXElementValidator.js index 044f9a2b05948..947d806058732 100644 --- a/packages/react/src/jsx/ReactJSXElementValidator.js +++ b/packages/react/src/jsx/ReactJSXElementValidator.js @@ -21,7 +21,6 @@ import { REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE, } from 'shared/ReactSymbols'; -import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags'; import hasOwnProperty from 'shared/hasOwnProperty'; import isArray from 'shared/isArray'; import {jsxDEV} from './ReactJSXElement'; @@ -390,31 +389,29 @@ export function jsxWithValidation( } } - if (warnAboutSpreadingKeyToJSX) { - if (hasOwnProperty.call(props, 'key')) { - const componentName = getComponentNameFromType(type); - const keys = Object.keys(props).filter(k => k !== 'key'); - const beforeExample = - keys.length > 0 - ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' - : '{key: someKey}'; - if (!didWarnAboutKeySpread[componentName + beforeExample]) { - const afterExample = - keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; - console.error( - 'A props object containing a "key" prop is being spread into JSX:\n' + - ' let props = %s;\n' + - ' <%s {...props} />\n' + - 'React keys must be passed directly to JSX without using spread:\n' + - ' let props = %s;\n' + - ' <%s key={someKey} {...props} />', - beforeExample, - componentName, - afterExample, - componentName, - ); - didWarnAboutKeySpread[componentName + beforeExample] = true; - } + if (hasOwnProperty.call(props, 'key')) { + const componentName = getComponentNameFromType(type); + const keys = Object.keys(props).filter(k => k !== 'key'); + const beforeExample = + keys.length > 0 + ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' + : '{key: someKey}'; + if (!didWarnAboutKeySpread[componentName + beforeExample]) { + const afterExample = + keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; + console.error( + 'A props object containing a "key" prop is being spread into JSX:\n' + + ' let props = %s;\n' + + ' <%s {...props} />\n' + + 'React keys must be passed directly to JSX without using spread:\n' + + ' let props = %s;\n' + + ' <%s key={someKey} {...props} />', + beforeExample, + componentName, + afterExample, + componentName, + ); + didWarnAboutKeySpread[componentName + beforeExample] = true; } } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 63d1aae161181..ad0aca1698dd3 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -196,7 +196,6 @@ export const disableTextareaChildren = false; // Enables a warning when trying to spread a 'key' to an element; // a deprecated pattern we want to get rid of in the future -export const warnAboutSpreadingKeyToJSX = true; // ----------------------------------------------------------------------------- // Debugging and DevTools diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 95a330b576c12..136a920fafe93 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -45,7 +45,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = false; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index d8a1d8c5d7427..8441d9476804a 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = false; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 92a4ecaf5dfad..ffb690e6d2926 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = false; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index dd2c58d33f4ec..79cb7c6de7c4d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = false; -export const warnAboutSpreadingKeyToJSX = true; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 78652d8835bb2..42c2d95c88e3b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = true; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 72a1a74523dc0..29185c128657f 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = false; export const disableModulePatternComponents = false; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index d5bc91306df4a..f1e39456d7e9f 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = __EXPERIMENTAL__; export const disableModulePatternComponents = true; -export const warnAboutSpreadingKeyToJSX = true; export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallbackFizz = false; export const enableCPUSuspense = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 0297c7dd44fcd..07cf0c774cc77 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -13,7 +13,6 @@ // Use __VARIANT__ to simulate a GK. The tests will be run twice: once // with the __VARIANT__ set to `true`, and once set to `false`. -export const warnAboutSpreadingKeyToJSX = __VARIANT__; export const disableInputAttributeSyncing = __VARIANT__; export const enableFilterEmptyStringAttributesDOM = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 76ea71d342a32..ffde17248b632 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -17,7 +17,6 @@ const dynamicFeatureFlags: DynamicFeatureFlags = require('ReactFeatureFlags'); export const { disableInputAttributeSyncing, disableSchedulerTimeoutBasedOnReactExpirationTime, - warnAboutSpreadingKeyToJSX, replayFailedUnitOfWorkWithInvokeGuardedCallback, enableFilterEmptyStringAttributesDOM, enableLegacyFBSupport, From 9afd752993923f0da9b682d91a371a19042d62c0 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 30 Jan 2023 11:43:00 -0500 Subject: [PATCH 2/2] remove old comment --- packages/shared/ReactFeatureFlags.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index ad0aca1698dd3..7a9a02262e9cf 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -185,18 +185,6 @@ export const enableCustomElementPropertySupport = __EXPERIMENTAL__; // Disables children for