From b43eec7eaad14747d24ef24a06b27cb2a5653bbc Mon Sep 17 00:00:00 2001 From: Laura buns Date: Fri, 6 Dec 2019 18:25:54 +0000 Subject: [PATCH] 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 --- .eslintrc.js | 1 + .../src/createSubscription.js | 18 +- packages/legacy-events/SyntheticEvent.js | 23 +- packages/react-dom/src/client/ReactDOM.js | 20 +- .../react-dom/src/client/ReactDOMSelect.js | 48 ++-- .../src/server/ReactPartialRenderer.js | 14 +- .../src/server/ReactPartialRendererHooks.js | 18 +- .../src/shared/CSSPropertyOperations.js | 58 ++--- .../src/shared/ReactDOMInvalidARIAHook.js | 156 ++++++------ .../shared/ReactDOMNullInputValuePropHook.js | 44 ++-- .../src/shared/ReactDOMUnknownPropertyHook.js | 61 +++-- packages/react-dom/src/shared/sanitizeURL.js | 20 +- .../src/test-utils/ReactTestUtils.js | 18 +- .../src/NativeMethodsMixin.js | 34 +-- .../react-native-renderer/src/ReactFabric.js | 12 +- .../src/ReactFabricHostConfig.js | 20 +- .../src/ReactNativeComponent.js | 34 +-- .../src/ReactNativeFiberHostComponent.js | 10 +- .../src/ReactNativeRenderer.js | 12 +- .../react-reconciler/src/ReactChildFiber.js | 32 +-- .../src/ReactFiberBeginWork.js | 128 +++++----- .../src/ReactFiberDevToolsHook.js | 32 +-- .../src/ReactFiberReconciler.js | 14 +- .../src/ReactFiberWorkLoop.js | 14 +- .../src/ReactShallowRenderer.js | 44 ++-- .../src/ReactTestHostConfig.js | 16 +- packages/react/src/ReactElement.js | 44 ++-- packages/react/src/ReactElementValidator.js | 188 +++++++------- ...lift-warning-conditional-argument-test.js} | 8 +- ...s => lift-warning-conditional-argument.js} | 17 +- .../no-production-logging-test.internal.js | 234 ++++++++++++++++++ scripts/eslint-rules/index.js | 1 + scripts/eslint-rules/no-production-logging.js | 72 ++++++ scripts/jest/preprocessor.js | 2 +- scripts/rollup/build.js | 6 +- 35 files changed, 928 insertions(+), 545 deletions(-) rename scripts/babel/__tests__/{wrap-warning-with-env-check-test.js => lift-warning-conditional-argument-test.js} (78%) rename scripts/babel/{wrap-warning-with-env-check.js => lift-warning-conditional-argument.js} (76%) create mode 100644 scripts/eslint-rules/__tests__/no-production-logging-test.internal.js create mode 100644 scripts/eslint-rules/no-production-logging.js diff --git a/.eslintrc.js b/.eslintrc.js index df147c90bd3e6..e0ace978a15da 100644 --- 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 index 38febd017cae8..ef22cdde2b0fc 100644 --- a/packages/create-subscription/src/createSubscription.js +++ b/packages/create-subscription/src/createSubscription.js @@ -36,14 +36,16 @@ export function createSubscription( }> { 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, diff --git a/packages/legacy-events/SyntheticEvent.js b/packages/legacy-events/SyntheticEvent.js index 78812c15db561..e569280ca3268 100644 --- 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 index b27de7a241d37..a6aa1e176cda6 100644 --- 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 index 3bc982d77c6a7..b688906ca099b 100644 --- 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 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 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 index d6310caffadf2..3bb7692e4ba3c 100644 --- 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 index d054caf53012f..86a9ac0a56a53 100644 --- 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( diff --git a/packages/react-dom/src/shared/CSSPropertyOperations.js b/packages/react-dom/src/shared/CSSPropertyOperations.js index 38589fd19daa3..29ae8cb737d97 100644 --- 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, - ); } } } diff --git a/packages/react-dom/src/shared/ReactDOMInvalidARIAHook.js b/packages/react-dom/src/shared/ReactDOMInvalidARIAHook.js index 59a7720c0a19b..b4973451c5bd3 100644 --- a/packages/react-dom/src/shared/ReactDOMInvalidARIAHook.js +++ b/packages/react-dom/src/shared/ReactDOMInvalidARIAHook.js @@ -18,62 +18,64 @@ const rARIACamel = new RegExp('^(aria)[A-Z][' + ATTRIBUTE_NAME_CHAR + ']*$'); const hasOwnProperty = Object.prototype.hasOwnProperty; function validateProperty(tagName, name) { - if (hasOwnProperty.call(warnedProperties, name) && warnedProperties[name]) { - return true; - } - - if (rARIACamel.test(name)) { - const ariaName = 'aria-' + name.slice(4).toLowerCase(); - const correctName = validAriaProperties.hasOwnProperty(ariaName) - ? ariaName - : null; - - // If this is an aria-* attribute, but is not listed in the known DOM - // DOM properties, then it is an invalid aria-* attribute. - if (correctName == null) { - warning( - false, - 'Invalid ARIA attribute `%s`. ARIA attributes follow the pattern aria-* and must be lowercase.', - name, - ); - warnedProperties[name] = true; + if (__DEV__) { + if (hasOwnProperty.call(warnedProperties, name) && warnedProperties[name]) { return true; } - // aria-* attributes should be lowercase; suggest the lowercase version. - if (name !== correctName) { - warning( - false, - 'Invalid ARIA attribute `%s`. Did you mean `%s`?', - name, - correctName, - ); - warnedProperties[name] = true; - return true; - } - } - if (rARIA.test(name)) { - const lowerCasedName = name.toLowerCase(); - const standardName = validAriaProperties.hasOwnProperty(lowerCasedName) - ? lowerCasedName - : null; + if (rARIACamel.test(name)) { + const ariaName = 'aria-' + name.slice(4).toLowerCase(); + const correctName = validAriaProperties.hasOwnProperty(ariaName) + ? ariaName + : null; - // If this is an aria-* attribute, but is not listed in the known DOM - // DOM properties, then it is an invalid aria-* attribute. - if (standardName == null) { - warnedProperties[name] = true; - return false; + // If this is an aria-* attribute, but is not listed in the known DOM + // DOM properties, then it is an invalid aria-* attribute. + if (correctName == null) { + warning( + false, + 'Invalid ARIA attribute `%s`. ARIA attributes follow the pattern aria-* and must be lowercase.', + name, + ); + warnedProperties[name] = true; + return true; + } + // aria-* attributes should be lowercase; suggest the lowercase version. + if (name !== correctName) { + warning( + false, + 'Invalid ARIA attribute `%s`. Did you mean `%s`?', + name, + correctName, + ); + warnedProperties[name] = true; + return true; + } } - // aria-* attributes should be lowercase; suggest the lowercase version. - if (name !== standardName) { - warning( - false, - 'Unknown ARIA attribute `%s`. Did you mean `%s`?', - name, - standardName, - ); - warnedProperties[name] = true; - return true; + + if (rARIA.test(name)) { + const lowerCasedName = name.toLowerCase(); + const standardName = validAriaProperties.hasOwnProperty(lowerCasedName) + ? lowerCasedName + : null; + + // If this is an aria-* attribute, but is not listed in the known DOM + // DOM properties, then it is an invalid aria-* attribute. + if (standardName == null) { + warnedProperties[name] = true; + return false; + } + // aria-* attributes should be lowercase; suggest the lowercase version. + if (name !== standardName) { + warning( + false, + 'Unknown ARIA attribute `%s`. Did you mean `%s`?', + name, + standardName, + ); + warnedProperties[name] = true; + return true; + } } } @@ -81,35 +83,37 @@ function validateProperty(tagName, name) { } function warnInvalidARIAProps(type, props) { - const invalidProps = []; + if (__DEV__) { + const invalidProps = []; - for (const key in props) { - const isValid = validateProperty(type, key); - if (!isValid) { - invalidProps.push(key); + for (const key in props) { + const isValid = validateProperty(type, key); + if (!isValid) { + invalidProps.push(key); + } } - } - const unknownPropString = invalidProps - .map(prop => '`' + prop + '`') - .join(', '); + const unknownPropString = invalidProps + .map(prop => '`' + prop + '`') + .join(', '); - if (invalidProps.length === 1) { - warning( - false, - 'Invalid aria prop %s on <%s> tag. ' + - 'For details, see https://fb.me/invalid-aria-prop', - unknownPropString, - type, - ); - } else if (invalidProps.length > 1) { - warning( - false, - 'Invalid aria props %s on <%s> tag. ' + - 'For details, see https://fb.me/invalid-aria-prop', - unknownPropString, - type, - ); + if (invalidProps.length === 1) { + warning( + false, + 'Invalid aria prop %s on <%s> tag. ' + + 'For details, see https://fb.me/invalid-aria-prop', + unknownPropString, + type, + ); + } else if (invalidProps.length > 1) { + warning( + false, + 'Invalid aria props %s on <%s> tag. ' + + 'For details, see https://fb.me/invalid-aria-prop', + unknownPropString, + type, + ); + } } } diff --git a/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js b/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js index 95f407b7da0c5..a3a36bd2be2da 100644 --- a/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js +++ b/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js @@ -10,28 +10,30 @@ import warning from 'shared/warning'; let didWarnValueNull = false; export function validateProperties(type, props) { - if (type !== 'input' && type !== 'textarea' && type !== 'select') { - return; - } + if (__DEV__) { + if (type !== 'input' && type !== 'textarea' && type !== 'select') { + return; + } - if (props != null && props.value === null && !didWarnValueNull) { - didWarnValueNull = true; - if (type === 'select' && props.multiple) { - warning( - false, - '`value` prop on `%s` should not be null. ' + - 'Consider using an empty array when `multiple` is set to `true` ' + - 'to clear the component or `undefined` for uncontrolled components.', - type, - ); - } else { - warning( - false, - '`value` prop on `%s` should not be null. ' + - 'Consider using an empty string to clear the component or `undefined` ' + - 'for uncontrolled components.', - type, - ); + if (props != null && props.value === null && !didWarnValueNull) { + didWarnValueNull = true; + if (type === 'select' && props.multiple) { + warning( + false, + '`value` prop on `%s` should not be null. ' + + 'Consider using an empty array when `multiple` is set to `true` ' + + 'to clear the component or `undefined` for uncontrolled components.', + type, + ); + } else { + warning( + false, + '`value` prop on `%s` should not be null. ' + + 'Consider using an empty string to clear the component or `undefined` ' + + 'for uncontrolled components.', + type, + ); + } } } } diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js index 3498111261857..b4ae9ee3d85f5 100644 --- a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js @@ -255,35 +255,42 @@ if (__DEV__) { } const warnUnknownProperties = function(type, props, canUseEventSystem) { - const unknownProps = []; - for (const key in props) { - const isValid = validateProperty(type, key, props[key], canUseEventSystem); - if (!isValid) { - unknownProps.push(key); + if (__DEV__) { + const unknownProps = []; + for (const key in props) { + const isValid = validateProperty( + type, + key, + props[key], + canUseEventSystem, + ); + if (!isValid) { + unknownProps.push(key); + } } - } - const unknownPropString = unknownProps - .map(prop => '`' + prop + '`') - .join(', '); - if (unknownProps.length === 1) { - warning( - false, - 'Invalid value for prop %s on <%s> tag. Either remove it from the element, ' + - 'or pass a string or number value to keep it in the DOM. ' + - 'For details, see https://fb.me/react-attribute-behavior', - unknownPropString, - type, - ); - } else if (unknownProps.length > 1) { - warning( - false, - 'Invalid values for props %s on <%s> tag. Either remove them from the element, ' + - 'or pass a string or number value to keep them in the DOM. ' + - 'For details, see https://fb.me/react-attribute-behavior', - unknownPropString, - type, - ); + const unknownPropString = unknownProps + .map(prop => '`' + prop + '`') + .join(', '); + if (unknownProps.length === 1) { + warning( + false, + 'Invalid value for prop %s on <%s> tag. Either remove it from the element, ' + + 'or pass a string or number value to keep it in the DOM. ' + + 'For details, see https://fb.me/react-attribute-behavior', + unknownPropString, + type, + ); + } else if (unknownProps.length > 1) { + warning( + false, + 'Invalid values for props %s on <%s> tag. Either remove them from the element, ' + + 'or pass a string or number value to keep them in the DOM. ' + + 'For details, see https://fb.me/react-attribute-behavior', + unknownPropString, + type, + ); + } } }; diff --git a/packages/react-dom/src/shared/sanitizeURL.js b/packages/react-dom/src/shared/sanitizeURL.js index 9e1034c1fe459..eda801a636f0a 100644 --- a/packages/react-dom/src/shared/sanitizeURL.js +++ b/packages/react-dom/src/shared/sanitizeURL.js @@ -38,15 +38,17 @@ function sanitizeURL(url: string) { 'React has blocked a javascript: URL as a security precaution.%s', __DEV__ ? ReactDebugCurrentFrame.getStackAddendum() : '', ); - } else if (__DEV__ && !didWarn && isJavaScriptProtocol.test(url)) { - didWarn = true; - warning( - false, - 'A future version of React will block javascript: URLs as a security precaution. ' + - 'Use event handlers instead if you can. If you need to generate unsafe HTML try ' + - 'using dangerouslySetInnerHTML instead. React was passed %s.', - JSON.stringify(url), - ); + } else if (__DEV__) { + if (!didWarn && isJavaScriptProtocol.test(url)) { + didWarn = true; + warning( + false, + 'A future version of React will block javascript: URLs as a security precaution. ' + + 'Use event handlers instead if you can. If you need to generate unsafe HTML try ' + + 'using dangerouslySetInnerHTML instead. React was passed %s.', + JSON.stringify(url), + ); + } } } diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index 58bfb04f8a570..064687bebe7e8 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -359,14 +359,16 @@ const ReactTestUtils = { * @return {object} the ReactTestUtils object (for chaining) */ mockComponent: function(module, mockTagName) { - if (!hasWarnedAboutDeprecatedMockComponent) { - hasWarnedAboutDeprecatedMockComponent = true; - lowPriorityWarningWithoutStack( - false, - 'ReactTestUtils.mockComponent() is deprecated. ' + - 'Use shallow rendering or jest.mock() instead.\n\n' + - 'See https://fb.me/test-utils-mock-component for more information.', - ); + if (__DEV__) { + if (!hasWarnedAboutDeprecatedMockComponent) { + hasWarnedAboutDeprecatedMockComponent = true; + lowPriorityWarningWithoutStack( + false, + 'ReactTestUtils.mockComponent() is deprecated. ' + + 'Use shallow rendering or jest.mock() instead.\n\n' + + 'See https://fb.me/test-utils-mock-component for more information.', + ); + } } mockTagName = mockTagName || module.mockTagName || 'div'; diff --git a/packages/react-native-renderer/src/NativeMethodsMixin.js b/packages/react-native-renderer/src/NativeMethodsMixin.js index 27d928cee4c35..823ccd3fe55e2 100644 --- a/packages/react-native-renderer/src/NativeMethodsMixin.js +++ b/packages/react-native-renderer/src/NativeMethodsMixin.js @@ -179,12 +179,14 @@ export default function( } if (maybeInstance.canonical) { - warningWithoutStack( - false, - 'Warning: measureLayout on components using NativeMethodsMixin ' + - 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + - 'measureLayout must be called on a native ref. Consider using forwardRef.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: measureLayout on components using NativeMethodsMixin ' + + 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + + 'measureLayout must be called on a native ref. Consider using forwardRef.', + ); + } return; } else { let relativeNode; @@ -197,10 +199,12 @@ export default function( } if (relativeNode == null) { - warningWithoutStack( - false, - 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + ); + } return; } @@ -243,10 +247,12 @@ export default function( } if (maybeInstance.canonical) { - warningWithoutStack( - false, - 'Warning: setNativeProps is not currently supported in Fabric', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + } return; } diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index ba4c6306b7514..d8e9ec6d58357 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -165,11 +165,13 @@ const ReactFabric: ReactFabricType = { handle._nativeTag == null || handle._internalInstanceHandle == null; if (invalid) { - warningWithoutStack( - !invalid, - "dispatchCommand was called with a ref that isn't a " + - 'native component. Use React.forwardRef to get access to the underlying native component', - ); + if (__DEV__) { + warningWithoutStack( + !invalid, + "dispatchCommand was called with a ref that isn't a " + + 'native component. Use React.forwardRef to get access to the underlying native component', + ); + } return; } diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 0731867150beb..8c1afecaf8b96 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -159,10 +159,12 @@ class ReactFabricHostComponent { typeof relativeToNativeNode === 'number' || !(relativeToNativeNode instanceof ReactFabricHostComponent) ) { - warningWithoutStack( - false, - 'Warning: ref.measureLayout must be called with a ref to a native component.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a ref to a native component.', + ); + } return; } @@ -176,10 +178,12 @@ class ReactFabricHostComponent { } setNativeProps(nativeProps: Object) { - warningWithoutStack( - false, - 'Warning: setNativeProps is not currently supported in Fabric', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + } return; } diff --git a/packages/react-native-renderer/src/ReactNativeComponent.js b/packages/react-native-renderer/src/ReactNativeComponent.js index 52dfab958966b..d9eb73b4b8725 100644 --- a/packages/react-native-renderer/src/ReactNativeComponent.js +++ b/packages/react-native-renderer/src/ReactNativeComponent.js @@ -190,12 +190,14 @@ export default function( } if (maybeInstance.canonical) { - warningWithoutStack( - false, - 'Warning: measureLayout on components using NativeMethodsMixin ' + - 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + - 'measureLayout must be called on a native ref. Consider using forwardRef.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: measureLayout on components using NativeMethodsMixin ' + + 'or ReactNative.NativeComponent is not currently supported in Fabric. ' + + 'measureLayout must be called on a native ref. Consider using forwardRef.', + ); + } return; } else { let relativeNode; @@ -208,10 +210,12 @@ export default function( } if (relativeNode == null) { - warningWithoutStack( - false, - 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + ); + } return; } @@ -254,10 +258,12 @@ export default function( } if (maybeInstance.canonical) { - warningWithoutStack( - false, - 'Warning: setNativeProps is not currently supported in Fabric', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: setNativeProps is not currently supported in Fabric', + ); + } return; } diff --git a/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js b/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js index 4bee7c7e1089a..dfe9d12617a93 100644 --- a/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js +++ b/packages/react-native-renderer/src/ReactNativeFiberHostComponent.js @@ -85,10 +85,12 @@ class ReactNativeFiberHostComponent { } if (relativeNode == null) { - warningWithoutStack( - false, - 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', - ); + if (__DEV__) { + warningWithoutStack( + false, + 'Warning: ref.measureLayout must be called with a node handle or a ref to a native component.', + ); + } return; } diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 4e64cbc90207d..55c34304f4d19 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -172,11 +172,13 @@ const ReactNativeRenderer: ReactNativeType = { dispatchCommand(handle: any, command: string, args: Array) { if (handle._nativeTag == null) { - warningWithoutStack( - handle._nativeTag != null, - "dispatchCommand was called with a ref that isn't a " + - 'native component. Use React.forwardRef to get access to the underlying native component', - ); + if (__DEV__) { + warningWithoutStack( + handle._nativeTag != null, + "dispatchCommand was called with a ref that isn't a " + + 'native component. Use React.forwardRef to get access to the underlying native component', + ); + } return; } diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index fb94fe1df1466..d8bf5ba0e6725 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -232,23 +232,25 @@ function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) { } function warnOnFunctionType() { - const currentComponentErrorInfo = - 'Functions are not valid as a React child. This may happen if ' + - 'you return a Component instead of from render. ' + - 'Or maybe you meant to call this function rather than return it.' + - getCurrentFiberStackInDev(); + if (__DEV__) { + const currentComponentErrorInfo = + 'Functions are not valid as a React child. This may happen if ' + + 'you return a Component instead of from render. ' + + 'Or maybe you meant to call this function rather than return it.' + + getCurrentFiberStackInDev(); - if (ownerHasFunctionTypeWarning[currentComponentErrorInfo]) { - return; - } - ownerHasFunctionTypeWarning[currentComponentErrorInfo] = true; + if (ownerHasFunctionTypeWarning[currentComponentErrorInfo]) { + return; + } + ownerHasFunctionTypeWarning[currentComponentErrorInfo] = true; - warning( - false, - 'Functions are not valid as a React child. This may happen if ' + - 'you return a Component instead of from render. ' + - 'Or maybe you meant to call this function rather than return it.', - ); + warning( + false, + 'Functions are not valid as a React child. This may happen if ' + + 'you return a Component instead of from render. ' + + 'Or maybe you meant to call this function rather than return it.', + ); + } } // This wrapper function exists because I expect to clone the code in each path diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 1df6aa55bf7bf..678e1398d39c1 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -1397,80 +1397,82 @@ function mountIndeterminateComponent( } function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) { - if (Component) { - warningWithoutStack( - !Component.childContextTypes, - '%s(...): childContextTypes cannot be defined on a function component.', - Component.displayName || Component.name || 'Component', - ); - } - if (workInProgress.ref !== null) { - let info = ''; - const ownerName = getCurrentFiberOwnerNameInDevOrNull(); - if (ownerName) { - info += '\n\nCheck the render method of `' + ownerName + '`.'; + if (__DEV__) { + if (Component) { + warningWithoutStack( + !Component.childContextTypes, + '%s(...): childContextTypes cannot be defined on a function component.', + Component.displayName || Component.name || 'Component', + ); } + if (workInProgress.ref !== null) { + let info = ''; + const ownerName = getCurrentFiberOwnerNameInDevOrNull(); + if (ownerName) { + info += '\n\nCheck the render method of `' + ownerName + '`.'; + } - let warningKey = ownerName || workInProgress._debugID || ''; - const debugSource = workInProgress._debugSource; - if (debugSource) { - warningKey = debugSource.fileName + ':' + debugSource.lineNumber; - } - if (!didWarnAboutFunctionRefs[warningKey]) { - didWarnAboutFunctionRefs[warningKey] = true; - warning( - false, - 'Function components cannot be given refs. ' + - 'Attempts to access this ref will fail. ' + - 'Did you mean to use React.forwardRef()?%s', - info, - ); + let warningKey = ownerName || workInProgress._debugID || ''; + const debugSource = workInProgress._debugSource; + if (debugSource) { + warningKey = debugSource.fileName + ':' + debugSource.lineNumber; + } + if (!didWarnAboutFunctionRefs[warningKey]) { + didWarnAboutFunctionRefs[warningKey] = true; + warning( + false, + 'Function components cannot be given refs. ' + + 'Attempts to access this ref will fail. ' + + 'Did you mean to use React.forwardRef()?%s', + info, + ); + } } - } - if ( - warnAboutDefaultPropsOnFunctionComponents && - Component.defaultProps !== undefined - ) { - const componentName = getComponentName(Component) || 'Unknown'; + if ( + warnAboutDefaultPropsOnFunctionComponents && + Component.defaultProps !== undefined + ) { + const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) { - warningWithoutStack( - false, - '%s: Support for defaultProps will be removed from function components ' + - 'in a future major release. Use JavaScript default parameters instead.', - componentName, - ); - didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true; + if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) { + warningWithoutStack( + false, + '%s: Support for defaultProps will be removed from function components ' + + 'in a future major release. Use JavaScript default parameters instead.', + componentName, + ); + didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true; + } } - } - if (typeof Component.getDerivedStateFromProps === 'function') { - const componentName = getComponentName(Component) || 'Unknown'; + if (typeof Component.getDerivedStateFromProps === 'function') { + const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) { - warningWithoutStack( - false, - '%s: Function components do not support getDerivedStateFromProps.', - componentName, - ); - didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true; + if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) { + warningWithoutStack( + false, + '%s: Function components do not support getDerivedStateFromProps.', + componentName, + ); + didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true; + } } - } - if ( - typeof Component.contextType === 'object' && - Component.contextType !== null - ) { - const componentName = getComponentName(Component) || 'Unknown'; + if ( + typeof Component.contextType === 'object' && + Component.contextType !== null + ) { + const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) { - warningWithoutStack( - false, - '%s: Function components do not support contextType.', - componentName, - ); - didWarnAboutContextTypeOnFunctionComponent[componentName] = true; + if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) { + warningWithoutStack( + false, + '%s: Function components do not support contextType.', + componentName, + ); + didWarnAboutContextTypeOnFunctionComponent[componentName] = true; + } } } } diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.js index df3f93463e00c..cf8177e3f9fc5 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.js @@ -89,13 +89,15 @@ export function injectInternals(internals: Object): boolean { hook.onCommitFiberRoot(rendererID, root, undefined, didError); } } catch (err) { - if (__DEV__ && !hasLoggedError) { - hasLoggedError = true; - warningWithoutStack( - false, - 'React instrumentation encountered an error: %s', - err, - ); + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + warningWithoutStack( + false, + 'React instrumentation encountered an error: %s', + err, + ); + } } } }; @@ -103,13 +105,15 @@ export function injectInternals(internals: Object): boolean { try { hook.onCommitFiberUnmount(rendererID, fiber); } catch (err) { - if (__DEV__ && !hasLoggedError) { - hasLoggedError = true; - warningWithoutStack( - false, - 'React instrumentation encountered an error: %s', - err, - ); + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + warningWithoutStack( + false, + 'React instrumentation encountered an error: %s', + err, + ); + } } } }; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 6ebac0dace803..e33a35c26f63b 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -280,12 +280,14 @@ export function updateContainer( callback = callback === undefined ? null : callback; if (callback !== null) { - warningWithoutStack( - typeof callback === 'function', - 'render(...): Expected the last optional `callback` argument to be a ' + - 'function. Instead received: %s.', - callback, - ); + if (__DEV__) { + warningWithoutStack( + typeof callback === 'function', + 'render(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callback, + ); + } update.callback = callback; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8bf99ce1139bf..8273d92b3c7a9 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1089,12 +1089,14 @@ export function flushDiscreteUpdates() { (executionContext & (BatchedContext | RenderContext | CommitContext)) !== NoContext ) { - if (__DEV__ && (executionContext & RenderContext) !== NoContext) { - warning( - false, - 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' + - 'already rendering.', - ); + if (__DEV__) { + if ((executionContext & RenderContext) !== NoContext) { + warning( + false, + 'unstable_flushDiscreteUpdates: Cannot flush updates when React is ' + + 'already rendering.', + ); + } } // We're already rendering, so we can't synchronously flush pending work. // This is probably a nested event dispatch triggered by a lifecycle/effect, diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 6da6467698624..0394f6087ea94 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -61,29 +61,33 @@ function areHookInputsEqual( prevDeps: Array | null, ) { if (prevDeps === null) { - warning( - false, - '%s received a final argument during this render, but not during ' + - 'the previous render. Even though the final argument is optional, ' + - 'its type cannot change between renders.', - currentHookNameInDev, - ); + if (__DEV__) { + warning( + false, + '%s received a final argument during this render, but not during ' + + 'the previous render. Even though the final argument is optional, ' + + 'its type cannot change between renders.', + currentHookNameInDev, + ); + } return false; } - // Don't bother comparing lengths in prod because these arrays should be - // passed inline. - if (nextDeps.length !== prevDeps.length) { - warning( - false, - 'The final argument passed to %s changed size between renders. The ' + - 'order and size of this array must remain constant.\n\n' + - 'Previous: %s\n' + - 'Incoming: %s', - currentHookNameInDev, - `[${nextDeps.join(', ')}]`, - `[${prevDeps.join(', ')}]`, - ); + if (__DEV__) { + // Don't bother comparing lengths in prod because these arrays should be + // passed inline. + if (nextDeps.length !== prevDeps.length) { + warning( + false, + 'The final argument passed to %s changed size between renders. The ' + + 'order and size of this array must remain constant.\n\n' + + 'Previous: %s\n' + + 'Incoming: %s', + currentHookNameInDev, + `[${nextDeps.join(', ')}]`, + `[${prevDeps.join(', ')}]`, + ); + } } for (let i = 0; i < prevDeps.length && i < nextDeps.length; i++) { if (is(nextDeps[i], prevDeps[i])) { diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index a3b8bff0f1339..cc1250670277a 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -214,13 +214,15 @@ export function createTextInstance( hostContext: Object, internalInstanceHandle: Object, ): TextInstance { - if (__DEV__ && enableFlareAPI) { - warning( - hostContext !== EVENT_COMPONENT_CONTEXT, - 'validateDOMNesting: React event components cannot have text DOM nodes as children. ' + - 'Wrap the child text "%s" in an element.', - text, - ); + if (__DEV__) { + if (enableFlareAPI) { + warning( + hostContext !== EVENT_COMPONENT_CONTEXT, + 'validateDOMNesting: React event components cannot have text DOM nodes as children. ' + + 'Wrap the child text "%s" in an element.', + text, + ); + } } return { text, diff --git a/packages/react/src/ReactElement.js b/packages/react/src/ReactElement.js index e900527b53660..bb31c57a978d2 100644 --- a/packages/react/src/ReactElement.js +++ b/packages/react/src/ReactElement.js @@ -48,16 +48,18 @@ function hasValidKey(config) { function defineKeyPropWarningGetter(props, displayName) { const warnAboutAccessingKey = function() { - if (!specialPropKeyWarningShown) { - specialPropKeyWarningShown = true; - warningWithoutStack( - false, - '%s: `key` is not a prop. Trying to access it will result ' + - 'in `undefined` being returned. If you need to access the same ' + - 'value within the child component, you should pass it as a different ' + - 'prop. (https://fb.me/react-special-props)', - displayName, - ); + if (__DEV__) { + if (!specialPropKeyWarningShown) { + specialPropKeyWarningShown = true; + warningWithoutStack( + false, + '%s: `key` is not a prop. Trying to access it will result ' + + 'in `undefined` being returned. If you need to access the same ' + + 'value within the child component, you should pass it as a different ' + + 'prop. (https://fb.me/react-special-props)', + displayName, + ); + } } }; warnAboutAccessingKey.isReactWarning = true; @@ -69,16 +71,18 @@ function defineKeyPropWarningGetter(props, displayName) { function defineRefPropWarningGetter(props, displayName) { const warnAboutAccessingRef = function() { - if (!specialPropRefWarningShown) { - specialPropRefWarningShown = true; - warningWithoutStack( - false, - '%s: `ref` is not a prop. Trying to access it will result ' + - 'in `undefined` being returned. If you need to access the same ' + - 'value within the child component, you should pass it as a different ' + - 'prop. (https://fb.me/react-special-props)', - displayName, - ); + if (__DEV__) { + if (!specialPropRefWarningShown) { + specialPropRefWarningShown = true; + warningWithoutStack( + false, + '%s: `ref` is not a prop. Trying to access it will result ' + + 'in `undefined` being returned. If you need to access the same ' + + 'value within the child component, you should pass it as a different ' + + 'prop. (https://fb.me/react-special-props)', + displayName, + ); + } } }; warnAboutAccessingRef.isReactWarning = true; diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 954d4e3be7c14..e011e0b9fea52 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -194,49 +194,51 @@ function validateChildKeys(node, parentType) { * @param {ReactElement} element */ function validatePropTypes(element) { - const type = element.type; - if (type === null || type === undefined || typeof type === 'string') { - return; - } - const name = getComponentName(type); - let propTypes; - if (typeof type === 'function') { - propTypes = type.propTypes; - } else if ( - typeof type === 'object' && - (type.$$typeof === REACT_FORWARD_REF_TYPE || - // Note: Memo only checks outer props here. - // Inner props are checked in the reconciler. - type.$$typeof === REACT_MEMO_TYPE) - ) { - propTypes = type.propTypes; - } else { - return; - } - if (propTypes) { - setCurrentlyValidatingElement(element); - checkPropTypes( - propTypes, - element.props, - 'prop', - name, - ReactDebugCurrentFrame.getStackAddendum, - ); - setCurrentlyValidatingElement(null); - } else if (type.PropTypes !== undefined && !propTypesMisspellWarningShown) { - propTypesMisspellWarningShown = true; - warningWithoutStack( - false, - 'Component %s declared `PropTypes` instead of `propTypes`. Did you misspell the property assignment?', - name || 'Unknown', - ); - } - if (typeof type.getDefaultProps === 'function') { - warningWithoutStack( - type.getDefaultProps.isReactClassApproved, - 'getDefaultProps is only used on classic React.createClass ' + - 'definitions. Use a static property named `defaultProps` instead.', - ); + if (__DEV__) { + const type = element.type; + if (type === null || type === undefined || typeof type === 'string') { + return; + } + const name = getComponentName(type); + let propTypes; + if (typeof type === 'function') { + propTypes = type.propTypes; + } else if ( + typeof type === 'object' && + (type.$$typeof === REACT_FORWARD_REF_TYPE || + // Note: Memo only checks outer props here. + // Inner props are checked in the reconciler. + type.$$typeof === REACT_MEMO_TYPE) + ) { + propTypes = type.propTypes; + } else { + return; + } + if (propTypes) { + setCurrentlyValidatingElement(element); + checkPropTypes( + propTypes, + element.props, + 'prop', + name, + ReactDebugCurrentFrame.getStackAddendum, + ); + setCurrentlyValidatingElement(null); + } else if (type.PropTypes !== undefined && !propTypesMisspellWarningShown) { + propTypesMisspellWarningShown = true; + warningWithoutStack( + false, + 'Component %s declared `PropTypes` instead of `propTypes`. Did you misspell the property assignment?', + name || 'Unknown', + ); + } + if (typeof type.getDefaultProps === 'function') { + warningWithoutStack( + type.getDefaultProps.isReactClassApproved, + 'getDefaultProps is only used on classic React.createClass ' + + 'definitions. Use a static property named `defaultProps` instead.', + ); + } } } @@ -245,27 +247,29 @@ function validatePropTypes(element) { * @param {ReactElement} fragment */ function validateFragmentProps(fragment) { - setCurrentlyValidatingElement(fragment); + if (__DEV__) { + setCurrentlyValidatingElement(fragment); - const keys = Object.keys(fragment.props); - for (let i = 0; i < keys.length; i++) { - const key = keys[i]; - if (key !== 'children' && key !== 'key') { - warning( - false, - 'Invalid prop `%s` supplied to `React.Fragment`. ' + - 'React.Fragment can only have `key` and `children` props.', - key, - ); - break; + const keys = Object.keys(fragment.props); + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + if (key !== 'children' && key !== 'key') { + warning( + false, + 'Invalid prop `%s` supplied to `React.Fragment`. ' + + 'React.Fragment can only have `key` and `children` props.', + key, + ); + break; + } } - } - if (fragment.ref !== null) { - warning(false, 'Invalid attribute `ref` supplied to `React.Fragment`.'); - } + if (fragment.ref !== null) { + warning(false, 'Invalid attribute `ref` supplied to `React.Fragment`.'); + } - setCurrentlyValidatingElement(null); + setCurrentlyValidatingElement(null); + } } export function jsxWithValidation( @@ -313,14 +317,16 @@ export function jsxWithValidation( typeString = typeof type; } - warning( - false, - 'React.jsx: type is invalid -- expected a string (for ' + - 'built-in components) or a class/function (for composite ' + - 'components) but got: %s.%s', - typeString, - info, - ); + if (__DEV__) { + warning( + false, + 'React.jsx: type is invalid -- expected a string (for ' + + 'built-in components) or a class/function (for composite ' + + 'components) but got: %s.%s', + typeString, + info, + ); + } } const element = jsxDEV(type, props, key, source, self); @@ -350,12 +356,14 @@ export function jsxWithValidation( Object.freeze(children); } } else { - warning( - false, - 'React.jsx: Static children should always be an array. ' + - 'You are likely explicitly calling React.jsxs or React.jsxDEV. ' + - 'Use the Babel transform instead.', - ); + if (__DEV__) { + warning( + false, + '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); @@ -364,12 +372,14 @@ export function jsxWithValidation( } if (hasOwnProperty.call(props, 'key')) { - warning( - false, - '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__) { + warning( + false, + '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 (type === REACT_FRAGMENT_TYPE) { @@ -431,14 +441,16 @@ export function createElementWithValidation(type, props, children) { typeString = typeof type; } - warning( - false, - 'React.createElement: type is invalid -- expected a string (for ' + - 'built-in components) or a class/function (for composite ' + - 'components) but got: %s.%s', - typeString, - info, - ); + if (__DEV__) { + warning( + false, + 'React.createElement: type is invalid -- expected a string (for ' + + 'built-in components) or a class/function (for composite ' + + 'components) but got: %s.%s', + typeString, + info, + ); + } } const element = createElement.apply(this, arguments); diff --git a/scripts/babel/__tests__/wrap-warning-with-env-check-test.js b/scripts/babel/__tests__/lift-warning-conditional-argument-test.js similarity index 78% rename from scripts/babel/__tests__/wrap-warning-with-env-check-test.js rename to scripts/babel/__tests__/lift-warning-conditional-argument-test.js index 1a2a78520ff06..158c262b3416b 100644 --- a/scripts/babel/__tests__/wrap-warning-with-env-check-test.js +++ b/scripts/babel/__tests__/lift-warning-conditional-argument-test.js @@ -8,7 +8,7 @@ 'use strict'; let babel = require('@babel/core'); -let wrapWarningWithEnvCheck = require('../wrap-warning-with-env-check'); +let wrapWarningWithEnvCheck = require('../lift-warning-conditional-argument'); function transform(input) { return babel.transform(input, { @@ -23,7 +23,7 @@ function compare(input, output) { let oldEnv; -describe('wrap-warning-with-env-check', () => { +describe('lift-warning-conditional-argument', () => { beforeEach(() => { oldEnv = process.env.NODE_ENV; process.env.NODE_ENV = ''; @@ -36,14 +36,14 @@ describe('wrap-warning-with-env-check', () => { it('should wrap warning calls', () => { compare( "warning(condition, 'a %s b', 'c');", - "__DEV__ ? !condition ? warning(false, 'a %s b', 'c') : void 0 : void 0;" + "!condition ? warning(false, 'a %s b', 'c') : void 0;" ); }); it('should wrap warningWithoutStack calls', () => { compare( "warningWithoutStack(condition, 'a %s b', 'c');", - "__DEV__ ? !condition ? warningWithoutStack(false, 'a %s b', 'c') : void 0 : void 0;" + "!condition ? warningWithoutStack(false, 'a %s b', 'c') : void 0;" ); }); diff --git a/scripts/babel/wrap-warning-with-env-check.js b/scripts/babel/lift-warning-conditional-argument.js similarity index 76% rename from scripts/babel/wrap-warning-with-env-check.js rename to scripts/babel/lift-warning-conditional-argument.js index f40d95d9a430d..f67f2a5f3dc09 100644 --- a/scripts/babel/wrap-warning-with-env-check.js +++ b/scripts/babel/lift-warning-conditional-argument.js @@ -9,8 +9,6 @@ module.exports = function(babel, options) { const t = babel.types; - const DEV_EXPRESSION = t.identifier('__DEV__'); - const SEEN_SYMBOL = Symbol('expression.seen'); return { @@ -34,10 +32,8 @@ module.exports = function(babel, options) { // // into this: // - // if (__DEV__) { - // if (!condition) { - // warning(false, argument, argument); - // } + // if (!condition) { + // warning(false, argument, argument); // } // // The goal is to strip out warning calls entirely in production @@ -50,13 +46,8 @@ module.exports = function(babel, options) { newNode[SEEN_SYMBOL] = true; path.replaceWith( t.ifStatement( - DEV_EXPRESSION, - t.blockStatement([ - t.ifStatement( - t.unaryExpression('!', condition), - t.expressionStatement(newNode) - ), - ]) + t.unaryExpression('!', condition), + t.expressionStatement(newNode) ) ); } diff --git a/scripts/eslint-rules/__tests__/no-production-logging-test.internal.js b/scripts/eslint-rules/__tests__/no-production-logging-test.internal.js new file mode 100644 index 0000000000000..0c4ecbfe4ad24 --- /dev/null +++ b/scripts/eslint-rules/__tests__/no-production-logging-test.internal.js @@ -0,0 +1,234 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +const rule = require('../no-production-logging'); +const RuleTester = require('eslint').RuleTester; +const ruleTester = new RuleTester(); + +ruleTester.run('no-production-logging', rule, { + valid: [ + { + code: ` + if (__DEV__) { + warning(test, 'Oh no'); + } + `, + }, + { + code: ` + if (__DEV__) { + warningWithoutStack(test, 'Oh no'); + } + `, + }, + { + code: ` + if (__DEV__) { + lowPriorityWarning(test, 'Oh no'); + } + `, + }, + { + code: ` + if (__DEV__) { + lowPriorityWarningWithoutStack(test, 'Oh no'); + } + `, + }, + // This is OK too because it's wrapped outside: + { + code: ` + if (__DEV__) { + if (potato) { + while (true) { + warning(test, 'Oh no'); + } + } + }`, + }, + { + code: ` + var f; + if (__DEV__) { + f = function() { + if (potato) { + while (true) { + warning(test, 'Oh no'); + } + } + }; + }`, + }, + // Don't do anything with these: + { + code: 'normalFunctionCall(test);', + }, + { + code: 'invariant(test);', + }, + { + code: ` + if (__DEV__) { + normalFunctionCall(test); + } + `, + }, + // This is OK because of the outer if. + { + code: ` + if (__DEV__) { + if (foo) { + if (__DEV__) { + } else { + warning(test, 'Oh no'); + } + } + }`, + }, + ], + invalid: [ + { + code: 'warning(test);', + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: 'warningWithoutStack(test)', + errors: [ + { + message: `Wrap warningWithoutStack() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (potato) { + warningWithoutStack(test); + } + `, + errors: [ + { + message: `Wrap warningWithoutStack() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: 'lowPriorityWarning(test);', + errors: [ + { + message: `Wrap lowPriorityWarning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: 'lowPriorityWarningWithoutStack(test)', + errors: [ + { + message: `Wrap lowPriorityWarningWithoutStack() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (potato) { + lowPriorityWarningWithoutStack(test); + } + `, + errors: [ + { + message: `Wrap lowPriorityWarningWithoutStack() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (__DEV__ || potato && true) { + warning(test); + } + `, + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (banana && __DEV__ && potato && kitten) { + warning(test); + } + `, + // Technically this code is valid but we prefer + // explicit standalone __DEV__ blocks that stand out. + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (!__DEV__) { + warning(test); + } + `, + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (foo || x && __DEV__) { + warning(test); + } + `, + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (__DEV__) { + } else { + warning(test); + } + `, + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + { + code: ` + if (__DEV__) { + } else { + if (__DEV__) { + } else { + warning(test); + } + } + `, + errors: [ + { + message: `Wrap warning() in an "if (__DEV__) {}" check`, + }, + ], + }, + ], +}); diff --git a/scripts/eslint-rules/index.js b/scripts/eslint-rules/index.js index 5864f614d7711..eca9ad2545101 100644 --- a/scripts/eslint-rules/index.js +++ b/scripts/eslint-rules/index.js @@ -5,5 +5,6 @@ module.exports = { 'no-primitive-constructors': require('./no-primitive-constructors'), 'no-to-warn-dev-within-to-throw': require('./no-to-warn-dev-within-to-throw'), 'warning-and-invariant-args': require('./warning-and-invariant-args'), + 'no-production-logging': require('./no-production-logging'), }, }; diff --git a/scripts/eslint-rules/no-production-logging.js b/scripts/eslint-rules/no-production-logging.js new file mode 100644 index 0000000000000..3732cf2387b5a --- /dev/null +++ b/scripts/eslint-rules/no-production-logging.js @@ -0,0 +1,72 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +const LOGGER_FN_NAMES = [ + 'warning', + 'warningWithoutStack', + 'lowPriorityWarning', + 'lowPriorityWarningWithoutStack', +]; + +module.exports = function(context) { + function isInDEVBlock(node) { + let done = false; + while (!done) { + let parent = node.parent; + if (!parent) { + return false; + } + if ( + parent.type === 'IfStatement' && + node === parent.consequent && + parent.test.type === 'Identifier' && + // This is intentionally strict so we can + // see blocks of DEV-only code at once. + parent.test.name === '__DEV__' + ) { + return true; + } + node = parent; + } + } + + function report(node) { + context.report({ + node: node, + message: `Wrap {{identifier}}() in an "if (__DEV__) {}" check`, + data: { + identifier: node.callee.name, + }, + fix: function(fixer) { + return [ + fixer.insertTextBefore(node.parent, `if (__DEV__) {`), + fixer.insertTextAfter(node.parent, '}'), + ]; + }, + }); + } + + const isLoggerFunctionName = name => LOGGER_FN_NAMES.includes(name); + + return { + meta: { + fixable: 'code', + }, + CallExpression: function(node) { + if (!isLoggerFunctionName(node.callee.name)) { + return; + } + if (!isInDEVBlock(node)) { + report(node); + } + }, + }; +}; diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index b8daddd80aba4..7bd496b472a0f 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -17,7 +17,7 @@ const pathToBabelPluginDevWithCode = require.resolve( '../error-codes/transform-error-messages' ); const pathToBabelPluginWrapWarning = require.resolve( - '../babel/wrap-warning-with-env-check' + '../babel/lift-warning-conditional-argument' ); const pathToBabelPluginAsyncToGenerator = require.resolve( '@babel/plugin-transform-async-to-generator' diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index ff0f9760bde1d..a8c697491af12 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -125,7 +125,7 @@ function getBabelConfig(updateBabelOptions, bundleType, filename) { // Minify invariant messages require('../error-codes/transform-error-messages'), // Wrap warning() calls in a __DEV__ check so they are stripped from production. - require('../babel/wrap-warning-with-env-check'), + require('../babel/lift-warning-conditional-argument'), ]), }); case RN_OSS_DEV: @@ -142,7 +142,7 @@ function getBabelConfig(updateBabelOptions, bundleType, filename) { {noMinify: true}, ], // Wrap warning() calls in a __DEV__ check so they are stripped from production. - require('../babel/wrap-warning-with-env-check'), + require('../babel/lift-warning-conditional-argument'), ]), }); case UMD_DEV: @@ -158,7 +158,7 @@ function getBabelConfig(updateBabelOptions, bundleType, filename) { // Minify invariant messages require('../error-codes/transform-error-messages'), // Wrap warning() calls in a __DEV__ check so they are stripped from production. - require('../babel/wrap-warning-with-env-check'), + require('../babel/lift-warning-conditional-argument'), ]), }); default: