Skip to content

Commit

Permalink
[cleanup] fully roll out warnAboutSpreadingKeyToJSX (#26080)
Browse files Browse the repository at this point in the history 
I fully enabled this flag internally now and unless I see complications,
we should be able to clean this up in the code.
  • Loading branch information
@kassens
kassens authored Jan 30, 2023
1 parent 48b687f commit 1f5ce59
Show file tree
Hide file tree
Showing 13 changed files with 74 additions and 106 deletions.
61 changes: 28 additions & 33 deletions packages/react/src/ReactElementValidator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -365,45 +364,41 @@ 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);
}
}
}

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;
}
}

Expand Down
48 changes: 23 additions & 25 deletions packages/react/src/__tests__/ReactElementJSX-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
' <Child {...props} />\n' +
'React keys must be passed directly to JSX without using spread:\n' +
' let props = {prop: ...};\n' +
' <Child key={someKey} {...props} />',
);
});
}
}
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' +
' <Child {...props} />\n' +
'React keys must be passed directly to JSX without using spread:\n' +
' let props = {prop: ...};\n' +
' <Child key={someKey} {...props} />',
);
});

it('should not warn when unkeyed children are passed to jsxs', () => {
const container = document.createElement('div');
Expand Down
49 changes: 23 additions & 26 deletions packages/react/src/jsx/ReactJSXElementValidator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
}

Expand Down
13 changes: 0 additions & 13 deletions packages/shared/ReactFeatureFlags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,6 @@ export const enableCustomElementPropertySupport = __EXPERIMENTAL__;
// Disables children for <textarea> elements
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
// -----------------------------------------------------------------------------
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.www.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const {
disableInputAttributeSyncing,
enableTrustedTypesIntegration,
disableSchedulerTimeoutBasedOnReactExpirationTime,
warnAboutSpreadingKeyToJSX,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
enableFilterEmptyStringAttributesDOM,
enableLegacyFBSupport,
Expand Down

0 comments on commit 1f5ce59

Please sign in to comment.