From 6c600131f34368cf1181f4f321955b19e94eefb3 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 19 Feb 2020 13:08:15 +0000 Subject: [PATCH] Feature Flag for React.jsx` "spreading a key to jsx" warning 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. --- packages/react/src/ReactElementValidator.js | 25 +++++++++++++------ .../ReactElementJSX-test.internal.js | 7 +++++- packages/shared/ReactFeatureFlags.js | 8 ++++++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.persistent.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 11 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index fa00f30d47d91..f2cfd0bd9c3d1 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -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'; @@ -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, @@ -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. ', - ); + if (__DEV__) { + if (warnAboutSpreadingKeyToJSX) { + if ( + didWarnAboutSpreadingKey[name] !== true && + hasOwnProperty.call(props, 'key') + ) { + 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} />', + getComponentName(type) || 'ComponentName', + ); + } } } diff --git a/packages/react/src/__tests__/ReactElementJSX-test.internal.js b/packages/react/src/__tests__/ReactElementJSX-test.internal.js index fce448208c0dc..b9eedeaa9ab8a 100644 --- a/packages/react/src/__tests__/ReactElementJSX-test.internal.js +++ b/packages/react/src/__tests__/ReactElementJSX-test.internal.js @@ -32,6 +32,7 @@ describe('ReactElement.jsx', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.enableJSXTransformAPI = true; + ReactFeatureFlags.warnAboutSpreadingKeyToJSX = true; React = require('react'); ReactDOM = require('react-dom'); @@ -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. ', + 'E.g. ', + ); + // does not warn twice for the same component + expect(() => ReactDOM.render(React.jsx(Parent, {}), container)).toErrorDev( + [], ); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index e69fee083ab55..3cecddefe4863 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -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. @@ -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 // -------------------------- diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index e0d5f0d111a95..196ba5a7b2836 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index a323c6b5ce27c..fa35bbb3b9340 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.persistent.js b/packages/shared/forks/ReactFeatureFlags.persistent.js index c1360b4164c89..6f90db361d8bc 100644 --- a/packages/shared/forks/ReactFeatureFlags.persistent.js +++ b/packages/shared/forks/ReactFeatureFlags.persistent.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b377ec18daf54..33e65fd5ab026 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 289aa51578689..d346df09739f4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index d3ffdb18c4c22..9e92e623a1c29 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 052f4e9d647cd..2d36299e164d7 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -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() { diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index c098e56b4166f..47d5ab3b4f41f 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -19,6 +19,7 @@ export const { runAllPassiveEffectDestroysBeforeCreates, warnAboutShorthandPropertyCollision, disableSchedulerTimeoutBasedOnReactExpirationTime, + warnAboutSpreadingKeyToJSX, } = require('ReactFeatureFlags'); // On WWW, __EXPERIMENTAL__ is used for a new modern build.