Replace wrap-warning-with-env-check with an eslint plugin (#17540)

* Replace Babel plugin with an ESLint plugin * Fix ESLint rule violations * Move shared conditions higher * Test formatting nits * Tweak ESLint rule * Bugfix: inside else branch, 'if' tests are not satisfactory * Use a stricter check for exactly if (__DEV__) This makes it easier to see what's going on and matches dominant style in the codebase. * Fix remaining files after stricter check
facebook · Dec 6, 2019 · b43eec7 · b43eec7
1 parent acfe4b2
commit b43eec7
Show file tree

Hide file tree

Showing 35 changed files with 928 additions and 545 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -93,6 +93,7 @@ module.exports = {
     'react-internal/no-primitive-constructors': ERROR,
     'react-internal/no-to-warn-dev-within-to-throw': ERROR,
     'react-internal/warning-and-invariant-args': ERROR,
+    'react-internal/no-production-logging': ERROR,
   },
 
   overrides: [

diff --git a/packages/create-subscription/src/createSubscription.js b/packages/create-subscription/src/createSubscription.js
@@ -36,14 +36,16 @@ export function createSubscription<Property, Value>(
 }> {
   const {getCurrentValue, subscribe} = config;
 
-  warningWithoutStack(
-    typeof getCurrentValue === 'function',
-    'Subscription must specify a getCurrentValue function',
-  );
-  warningWithoutStack(
-    typeof subscribe === 'function',
-    'Subscription must specify a subscribe function',
-  );
+  if (__DEV__) {
+    warningWithoutStack(
+      typeof getCurrentValue === 'function',
+      'Subscription must specify a getCurrentValue function',
+    );
+    warningWithoutStack(
+      typeof subscribe === 'function',
+      'Subscription must specify a subscribe function',
+    );
+  }
 
   type Props = {
     children: (value: Value) => React$Element<any>,

diff --git a/packages/legacy-events/SyntheticEvent.js b/packages/legacy-events/SyntheticEvent.js
@@ -283,17 +283,18 @@ function getPooledWarningPropertyDefinition(propName, getVal) {
   }
 
   function warn(action, result) {
-    const warningCondition = false;
-    warningWithoutStack(
-      warningCondition,
-      "This synthetic event is reused for performance reasons. If you're seeing this, " +
-        "you're %s `%s` on a released/nullified synthetic event. %s. " +
-        'If you must keep the original synthetic event around, use event.persist(). ' +
-        'See https://fb.me/react-event-pooling for more information.',
-      action,
-      propName,
-      result,
-    );
+    if (__DEV__) {
+      warningWithoutStack(
+        false,
+        "This synthetic event is reused for performance reasons. If you're seeing this, " +
+          "you're %s `%s` on a released/nullified synthetic event. %s. " +
+          'If you must keep the original synthetic event around, use event.persist(). ' +
+          'See https://fb.me/react-event-pooling for more information.',
+        action,
+        propName,
+        result,
+      );
+    }
   }
 }
 

diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js
@@ -142,15 +142,17 @@ const ReactDOM: Object = {
   // Temporary alias since we already shipped React 16 RC with it.
   // TODO: remove in React 17.
   unstable_createPortal(...args) {
-    if (!didWarnAboutUnstableCreatePortal) {
-      didWarnAboutUnstableCreatePortal = true;
-      lowPriorityWarningWithoutStack(
-        false,
-        'The ReactDOM.unstable_createPortal() alias has been deprecated, ' +
-          'and will be removed in React 17+. Update your code to use ' +
-          'ReactDOM.createPortal() instead. It has the exact same API, ' +
-          'but without the "unstable_" prefix.',
-      );
+    if (__DEV__) {
+      if (!didWarnAboutUnstableCreatePortal) {
+        didWarnAboutUnstableCreatePortal = true;
+        lowPriorityWarningWithoutStack(
+          false,
+          'The ReactDOM.unstable_createPortal() alias has been deprecated, ' +
+            'and will be removed in React 17+. Update your code to use ' +
+            'ReactDOM.createPortal() instead. It has the exact same API, ' +
+            'but without the "unstable_" prefix.',
+        );
+      }
     }
     return createPortal(...args);
   },

diff --git a/packages/react-dom/src/client/ReactDOMSelect.js b/packages/react-dom/src/client/ReactDOMSelect.js
@@ -40,30 +40,32 @@ const valuePropNames = ['value', 'defaultValue'];
  * Validation function for `value` and `defaultValue`.
  */
 function checkSelectPropTypes(props) {
-  ReactControlledValuePropTypes.checkPropTypes('select', props);
+  if (__DEV__) {
+    ReactControlledValuePropTypes.checkPropTypes('select', props);
 
-  for (let i = 0; i < valuePropNames.length; i++) {
-    const propName = valuePropNames[i];
-    if (props[propName] == null) {
-      continue;
-    }
-    const isArray = Array.isArray(props[propName]);
-    if (props.multiple && !isArray) {
-      warning(
-        false,
-        'The `%s` prop supplied to <select> must be an array if ' +
-          '`multiple` is true.%s',
-        propName,
-        getDeclarationErrorAddendum(),
-      );
-    } else if (!props.multiple && isArray) {
-      warning(
-        false,
-        'The `%s` prop supplied to <select> must be a scalar ' +
-          'value if `multiple` is false.%s',
-        propName,
-        getDeclarationErrorAddendum(),
-      );
+    for (let i = 0; i < valuePropNames.length; i++) {
+      const propName = valuePropNames[i];
+      if (props[propName] == null) {
+        continue;
+      }
+      const isArray = Array.isArray(props[propName]);
+      if (props.multiple && !isArray) {
+        warning(
+          false,
+          'The `%s` prop supplied to <select> must be an array if ' +
+            '`multiple` is true.%s',
+          propName,
+          getDeclarationErrorAddendum(),
+        );
+      } else if (!props.multiple && isArray) {
+        warning(
+          false,
+          'The `%s` prop supplied to <select> must be a scalar ' +
+            'value if `multiple` is false.%s',
+          propName,
+          getDeclarationErrorAddendum(),
+        );
+      }
     }
   }
 }

diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js
@@ -687,12 +687,14 @@ function resolve(
             );
           }
         } else {
-          warningWithoutStack(
-            false,
-            '%s.getChildContext(): childContextTypes must be defined in order to ' +
-              'use getChildContext().',
-            getComponentName(Component) || 'Unknown',
-          );
+          if (__DEV__) {
+            warningWithoutStack(
+              false,
+              '%s.getChildContext(): childContextTypes must be defined in order to ' +
+                'use getChildContext().',
+              getComponentName(Component) || 'Unknown',
+            );
+          }
         }
       }
       if (childContext) {

diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js
@@ -392,16 +392,16 @@ export function useLayoutEffect(
 ) {
   if (__DEV__) {
     currentHookNameInDev = 'useLayoutEffect';
+    warning(
+      false,
+      'useLayoutEffect does nothing on the server, because its effect cannot ' +
+        "be encoded into the server renderer's output format. This will lead " +
+        'to a mismatch between the initial, non-hydrated UI and the intended ' +
+        'UI. To avoid this, useLayoutEffect should only be used in ' +
+        'components that render exclusively on the client. ' +
+        'See https://fb.me/react-uselayouteffect-ssr for common fixes.',
+    );
   }
-  warning(
-    false,
-    'useLayoutEffect does nothing on the server, because its effect cannot ' +
-      "be encoded into the server renderer's output format. This will lead " +
-      'to a mismatch between the initial, non-hydrated UI and the intended ' +
-      'UI. To avoid this, useLayoutEffect should only be used in ' +
-      'components that render exclusively on the client. ' +
-      'See https://fb.me/react-uselayouteffect-ssr for common fixes.',
-  );
 }
 
 function dispatchAction<A>(

diff --git a/packages/react-dom/src/shared/CSSPropertyOperations.js b/packages/react-dom/src/shared/CSSPropertyOperations.js
@@ -128,37 +128,39 @@ export function validateShorthandPropertyCollisionInDev(
   styleUpdates,
   nextStyles,
 ) {
-  if (!warnAboutShorthandPropertyCollision) {
-    return;
-  }
+  if (__DEV__) {
+    if (!warnAboutShorthandPropertyCollision) {
+      return;
+    }
 
-  if (!nextStyles) {
-    return;
-  }
+    if (!nextStyles) {
+      return;
+    }
 
-  const expandedUpdates = expandShorthandMap(styleUpdates);
-  const expandedStyles = expandShorthandMap(nextStyles);
-  const warnedAbout = {};
-  for (const key in expandedUpdates) {
-    const originalKey = expandedUpdates[key];
-    const correctOriginalKey = expandedStyles[key];
-    if (correctOriginalKey && originalKey !== correctOriginalKey) {
-      const warningKey = originalKey + ',' + correctOriginalKey;
-      if (warnedAbout[warningKey]) {
-        continue;
+    const expandedUpdates = expandShorthandMap(styleUpdates);
+    const expandedStyles = expandShorthandMap(nextStyles);
+    const warnedAbout = {};
+    for (const key in expandedUpdates) {
+      const originalKey = expandedUpdates[key];
+      const correctOriginalKey = expandedStyles[key];
+      if (correctOriginalKey && originalKey !== correctOriginalKey) {
+        const warningKey = originalKey + ',' + correctOriginalKey;
+        if (warnedAbout[warningKey]) {
+          continue;
+        }
+        warnedAbout[warningKey] = true;
+        warning(
+          false,
+          '%s a style property during rerender (%s) when a ' +
+            'conflicting property is set (%s) can lead to styling bugs. To ' +
+            "avoid this, don't mix shorthand and non-shorthand properties " +
+            'for the same value; instead, replace the shorthand with ' +
+            'separate values.',
+          isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
+          originalKey,
+          correctOriginalKey,
+        );
       }
-      warnedAbout[warningKey] = true;
-      warning(
-        false,
-        '%s a style property during rerender (%s) when a ' +
-          'conflicting property is set (%s) can lead to styling bugs. To ' +
-          "avoid this, don't mix shorthand and non-shorthand properties " +
-          'for the same value; instead, replace the shorthand with ' +
-          'separate values.',
-        isValueEmpty(styleUpdates[originalKey]) ? 'Removing' : 'Updating',
-        originalKey,
-        correctOriginalKey,
-      );
     }
   }
 }