Skip to content

Commit

Permalink
Feature Flag for React.jsx` "spreading a key to jsx" warning
Browse files Browse the repository at this point in the history
Adds a feature flag for when React.jsx warns you about spreading a key into jsx. It's false for all builds, except as a dynamic flag for fb/www.

I also made it so it warns at most once per component name, and included the name in the warning.
  • Loading branch information
threepointone committed Feb 19, 2020
1 parent 4d9f850 commit 78f69d0
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 8 deletions.
25 changes: 18 additions & 7 deletions packages/react/src/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
REACT_FRAGMENT_TYPE,
REACT_ELEMENT_TYPE,
} from 'shared/ReactSymbols';
import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags';
import checkPropTypes from 'prop-types/checkPropTypes';

import ReactCurrentOwner from './ReactCurrentOwner';
Expand Down Expand Up @@ -268,6 +269,9 @@ function validateFragmentProps(fragment) {
}
}

// warn about spreading keys to an element only once per ComponentName
const didWarnAboutSpreadingKey = {};

export function jsxWithValidation(
type,
props,
Expand Down Expand Up @@ -365,13 +369,20 @@ export function jsxWithValidation(
}
}

if (hasOwnProperty.call(props, 'key')) {
if (__DEV__) {
console.error(
'React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
'Explicitly pass a key after spreading props in your JSX call. ' +
'E.g. <ComponentName {...props} key={key} />',
);
if (__DEV__) {
if (warnAboutSpreadingKeyToJSX) {
if (hasOwnProperty.call(props, 'key')) {
const name = getComponentName(type) || 'ComponentName';
if (didWarnAboutSpreadingKey[name] !== true) {
didWarnAboutSpreadingKey[name] = true;
console.error(
'React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
'Explicitly pass a key after spreading props in your JSX call. ' +
'E.g. <%s {...props} key={key} />',
name,
);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('ReactElement.jsx', () => {

ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableJSXTransformAPI = true;
ReactFeatureFlags.warnAboutSpreadingKeyToJSX = true;

React = require('react');
ReactDOM = require('react-dom');
Expand Down Expand Up @@ -371,7 +372,11 @@ describe('ReactElement.jsx', () => {
expect(() => ReactDOM.render(React.jsx(Parent, {}), container)).toErrorDev(
'Warning: React.jsx: Spreading a key to JSX is a deprecated pattern. ' +
'Explicitly pass a key after spreading props in your JSX call. ' +
'E.g. <ComponentName {...props} key={key} />',
'E.g. <Child {...props} key={key} />',
);
// does not warn twice for the same component
expect(() => ReactDOM.render(React.jsx(Parent, {}), container)).toErrorDev(
[],
);
});

Expand Down
8 changes: 8 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export const disableJavaScriptURLs = false;
// Control this behavior with a flag to support 16.6 minor releases in the meanwhile.
export const exposeConcurrentModeAPIs = __EXPERIMENTAL__;

// Warns when a combination of updates on a dom can cause a style declaration
// that clashes with a previous one https://github.com/facebook/react/pull/14181
export const warnAboutShorthandPropertyCollision = true;

// Experimental React Flare event system and event components support.
Expand Down Expand Up @@ -106,8 +108,14 @@ export const runAllPassiveEffectDestroysBeforeCreates = false;
// WARNING This flag only has an affect if used with runAllPassiveEffectDestroysBeforeCreates.
export const deferPassiveEffectCleanupDuringUnmount = false;

// Use this flag to generate "testing" builds, that include APIs like act()
// and extra warnings/errors
export const isTestEnvironment = 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 = false;

// --------------------------
// Future APIs to be deprecated
// --------------------------
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = false;
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = false;
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = false;
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = true; // this should probably *never* change
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = true; // this should probably *never* change
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = false;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = true;
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const disableUnstableCreatePortal = __EXPERIMENTAL__;
export const deferPassiveEffectCleanupDuringUnmount = false;
export const runAllPassiveEffectDestroysBeforeCreates = false;
export const isTestEnvironment = true;
export const warnAboutSpreadingKeyToJSX = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const {
runAllPassiveEffectDestroysBeforeCreates,
warnAboutShorthandPropertyCollision,
disableSchedulerTimeoutBasedOnReactExpirationTime,
warnAboutSpreadingKeyToJSX,
} = require('ReactFeatureFlags');

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down

0 comments on commit 78f69d0

Please sign in to comment.