[cleanup] fully roll out warnAboutSpreadingKeyToJSX (#26080)

I fully enabled this flag internally now and unless I see complications,
we should be able to clean this up in the code.
This commit is contained in:
Jan Kassens 2023-01-30 15:25:24 -05:00 committed by GitHub
parent 48b687fc95
commit 1f5ce59dd7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 74 additions and 106 deletions

View File

@ -21,7 +21,6 @@ import {
REACT_FRAGMENT_TYPE, REACT_FRAGMENT_TYPE,
REACT_ELEMENT_TYPE, REACT_ELEMENT_TYPE,
} from 'shared/ReactSymbols'; } from 'shared/ReactSymbols';
import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags';
import checkPropTypes from 'shared/checkPropTypes'; import checkPropTypes from 'shared/checkPropTypes';
import isArray from 'shared/isArray'; import isArray from 'shared/isArray';
@ -365,13 +364,11 @@ export function jsxWithValidation(
Object.freeze(children); Object.freeze(children);
} }
} else { } else {
if (__DEV__) { console.error(
console.error( 'React.jsx: Static children should always be an array. ' +
'React.jsx: Static children should always be an array. ' + 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' +
'You are likely explicitly calling React.jsxs or React.jsxDEV. ' + 'Use the Babel transform instead.',
'Use the Babel transform instead.', );
);
}
} }
} else { } else {
validateChildKeys(children, type); validateChildKeys(children, type);
@ -379,31 +376,29 @@ export function jsxWithValidation(
} }
} }
if (warnAboutSpreadingKeyToJSX) { if (hasOwnProperty.call(props, 'key')) {
if (hasOwnProperty.call(props, 'key')) { const componentName = getComponentNameFromType(type);
const componentName = getComponentNameFromType(type); const keys = Object.keys(props).filter(k => k !== 'key');
const keys = Object.keys(props).filter(k => k !== 'key'); const beforeExample =
const beforeExample = keys.length > 0
keys.length > 0 ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}'
? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' : '{key: someKey}';
: '{key: someKey}'; if (!didWarnAboutKeySpread[componentName + beforeExample]) {
if (!didWarnAboutKeySpread[componentName + beforeExample]) { const afterExample =
const afterExample = keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}';
keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; console.error(
console.error( 'A props object containing a "key" prop is being spread into JSX:\n' +
'A props object containing a "key" prop is being spread into JSX:\n' + ' let props = %s;\n' +
' let props = %s;\n' + ' <%s {...props} />\n' +
' <%s {...props} />\n' + 'React keys must be passed directly to JSX without using spread:\n' +
'React keys must be passed directly to JSX without using spread:\n' + ' let props = %s;\n' +
' let props = %s;\n' + ' <%s key={someKey} {...props} />',
' <%s key={someKey} {...props} />', beforeExample,
beforeExample, componentName,
componentName, afterExample,
afterExample, componentName,
componentName, );
); didWarnAboutKeySpread[componentName + beforeExample] = true;
didWarnAboutKeySpread[componentName + beforeExample] = true;
}
} }
} }

View File

@ -264,33 +264,31 @@ describe('ReactElement.jsx', () => {
); );
}); });
if (require('shared/ReactFeatureFlags').warnAboutSpreadingKeyToJSX) { it('should warn when keys are passed as part of props', () => {
it('should warn when keys are passed as part of props', () => { const container = document.createElement('div');
const container = document.createElement('div'); class Child extends React.Component {
class Child extends React.Component { render() {
render() { return JSXRuntime.jsx('div', {});
return JSXRuntime.jsx('div', {});
}
} }
class Parent extends React.Component { }
render() { class Parent extends React.Component {
return JSXRuntime.jsx('div', { render() {
children: [JSXRuntime.jsx(Child, {key: '0', prop: 'hi'})], return JSXRuntime.jsx('div', {
}); children: [JSXRuntime.jsx(Child, {key: '0', prop: 'hi'})],
} });
} }
expect(() => }
ReactDOM.render(JSXRuntime.jsx(Parent, {}), container), expect(() =>
).toErrorDev( ReactDOM.render(JSXRuntime.jsx(Parent, {}), container),
'Warning: A props object containing a "key" prop is being spread into JSX:\n' + ).toErrorDev(
' let props = {key: someKey, prop: ...};\n' + 'Warning: A props object containing a "key" prop is being spread into JSX:\n' +
' <Child {...props} />\n' + ' let props = {key: someKey, prop: ...};\n' +
'React keys must be passed directly to JSX without using spread:\n' + ' <Child {...props} />\n' +
' let props = {prop: ...};\n' + 'React keys must be passed directly to JSX without using spread:\n' +
' <Child key={someKey} {...props} />', ' let props = {prop: ...};\n' +
); ' <Child key={someKey} {...props} />',
}); );
} });
it('should not warn when unkeyed children are passed to jsxs', () => { it('should not warn when unkeyed children are passed to jsxs', () => {
const container = document.createElement('div'); const container = document.createElement('div');

View File

@ -21,7 +21,6 @@ import {
REACT_FRAGMENT_TYPE, REACT_FRAGMENT_TYPE,
REACT_ELEMENT_TYPE, REACT_ELEMENT_TYPE,
} from 'shared/ReactSymbols'; } from 'shared/ReactSymbols';
import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags';
import hasOwnProperty from 'shared/hasOwnProperty'; import hasOwnProperty from 'shared/hasOwnProperty';
import isArray from 'shared/isArray'; import isArray from 'shared/isArray';
import {jsxDEV} from './ReactJSXElement'; import {jsxDEV} from './ReactJSXElement';
@ -390,31 +389,29 @@ export function jsxWithValidation(
} }
} }
if (warnAboutSpreadingKeyToJSX) { if (hasOwnProperty.call(props, 'key')) {
if (hasOwnProperty.call(props, 'key')) { const componentName = getComponentNameFromType(type);
const componentName = getComponentNameFromType(type); const keys = Object.keys(props).filter(k => k !== 'key');
const keys = Object.keys(props).filter(k => k !== 'key'); const beforeExample =
const beforeExample = keys.length > 0
keys.length > 0 ? '{key: someKey, ' + keys.join(': ..., ') + ': ...}'
? '{key: someKey, ' + keys.join(': ..., ') + ': ...}' : '{key: someKey}';
: '{key: someKey}'; if (!didWarnAboutKeySpread[componentName + beforeExample]) {
if (!didWarnAboutKeySpread[componentName + beforeExample]) { const afterExample =
const afterExample = keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}';
keys.length > 0 ? '{' + keys.join(': ..., ') + ': ...}' : '{}'; console.error(
console.error( 'A props object containing a "key" prop is being spread into JSX:\n' +
'A props object containing a "key" prop is being spread into JSX:\n' + ' let props = %s;\n' +
' let props = %s;\n' + ' <%s {...props} />\n' +
' <%s {...props} />\n' + 'React keys must be passed directly to JSX without using spread:\n' +
'React keys must be passed directly to JSX without using spread:\n' + ' let props = %s;\n' +
' let props = %s;\n' + ' <%s key={someKey} {...props} />',
' <%s key={someKey} {...props} />', beforeExample,
beforeExample, componentName,
componentName, afterExample,
afterExample, componentName,
componentName, );
); didWarnAboutKeySpread[componentName + beforeExample] = true;
didWarnAboutKeySpread[componentName + beforeExample] = true;
}
} }
} }

View File

@ -185,19 +185,6 @@ export const enableCustomElementPropertySupport = __EXPERIMENTAL__;
// Disables children for <textarea> elements // Disables children for <textarea> elements
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
// -----------------------------------------------------------------------------
// JSX Chopping Block
//
// Similar to main Chopping Block but only flags related to JSX. These are
// grouped because we will likely batch all of them into a single major release.
// -----------------------------------------------------------------------------
// New API for JSX transforms to target - https://github.com/reactjs/rfcs/pull/107
// 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 // Debugging and DevTools
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------

View File

@ -45,7 +45,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = false; export const disableModulePatternComponents = false;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = true; export const enableCPUSuspense = true;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = false; export const disableModulePatternComponents = false;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = false; export const enableCPUSuspense = false;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = false; export const disableModulePatternComponents = false;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = false; export const enableCPUSuspense = false;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = false; export const disableModulePatternComponents = false;
export const warnAboutSpreadingKeyToJSX = true;
export const enableComponentStackLocations = false; export const enableComponentStackLocations = false;
export const enableLegacyFBSupport = false; export const enableLegacyFBSupport = false;
export const enableFilterEmptyStringAttributesDOM = false; export const enableFilterEmptyStringAttributesDOM = false;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = true; export const disableModulePatternComponents = true;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallback = true;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = false; export const enableCPUSuspense = false;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = false; export const disableTextareaChildren = false;
export const disableModulePatternComponents = false; export const disableModulePatternComponents = false;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseAvoidThisFallback = false;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = false; export const enableCPUSuspense = false;

View File

@ -35,7 +35,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false; export const enableTrustedTypesIntegration = false;
export const disableTextareaChildren = __EXPERIMENTAL__; export const disableTextareaChildren = __EXPERIMENTAL__;
export const disableModulePatternComponents = true; export const disableModulePatternComponents = true;
export const warnAboutSpreadingKeyToJSX = true;
export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallback = true;
export const enableSuspenseAvoidThisFallbackFizz = false; export const enableSuspenseAvoidThisFallbackFizz = false;
export const enableCPUSuspense = true; export const enableCPUSuspense = true;

View File

@ -13,7 +13,6 @@
// Use __VARIANT__ to simulate a GK. The tests will be run twice: once // Use __VARIANT__ to simulate a GK. The tests will be run twice: once
// with the __VARIANT__ set to `true`, and once set to `false`. // with the __VARIANT__ set to `true`, and once set to `false`.
export const warnAboutSpreadingKeyToJSX = __VARIANT__;
export const disableInputAttributeSyncing = __VARIANT__; export const disableInputAttributeSyncing = __VARIANT__;
export const enableFilterEmptyStringAttributesDOM = __VARIANT__; export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
export const enableLegacyFBSupport = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__;

View File

@ -18,7 +18,6 @@ export const {
disableInputAttributeSyncing, disableInputAttributeSyncing,
enableTrustedTypesIntegration, enableTrustedTypesIntegration,
disableSchedulerTimeoutBasedOnReactExpirationTime, disableSchedulerTimeoutBasedOnReactExpirationTime,
warnAboutSpreadingKeyToJSX,
replayFailedUnitOfWorkWithInvokeGuardedCallback, replayFailedUnitOfWorkWithInvokeGuardedCallback,
enableFilterEmptyStringAttributesDOM, enableFilterEmptyStringAttributesDOM,
enableLegacyFBSupport, enableLegacyFBSupport,