Skip to content

Commit

Permalink
Replace wrap-warning-with-env-check with an eslint plugin (#17540)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Laura buns authored and gaearon committed Dec 6, 2019
1 parent acfe4b2 commit b43eec7
Show file tree
Hide file tree
Showing 35 changed files with 928 additions and 545 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down
18 changes: 10 additions & 8 deletions packages/create-subscription/src/createSubscription.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
23 changes: 12 additions & 11 deletions packages/legacy-events/SyntheticEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}
}

Expand Down
20 changes: 11 additions & 9 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down
48 changes: 25 additions & 23 deletions packages/react-dom/src/client/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
}
}
}
}
Expand Down
14 changes: 8 additions & 6 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions packages/react-dom/src/server/ReactPartialRendererHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
58 changes: 30 additions & 28 deletions packages/react-dom/src/shared/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}
}
Loading

0 comments on commit b43eec7

Please sign in to comment.