oliviertassinari / babel-plugin-transform-react-remove-prop-types

Remove unnecessary React propTypes from the production build. :balloon:
MIT License
897 stars 61 forks source link

Destructured PropTypes are not removed #183

Closed ghost closed 5 years ago

ghost commented 5 years ago

When PropTypes are destructured, they are ignored by the plugin. For example, we have a UI library that extends Material-UI. One of its components is a button, so we'd like to inherit the default propTypes and augment them with our own:

import { Button as MuiButton } from '@material-ui/core'

const Button = props => /* ... */

Button.propTypes = {
  ...MuiButton.propTypes,
  other: PropTypes.string // etc.
}

Button.defaultProps = {
  ...MuiButton.defaultProps,
  other: '' // etc.
}

Expected result:

process.env.NODE_ENV !== "production" ? Button.propTypes = _objectSpread({other: PropTypes.string}, MuiButton.propTypes) : void 0;
Button.defaultProps = _objectSpread({other: ''}, MuiButton.defaultProps);

Actual result:

Button.propTypes = _objectSpread({other: PropTypes.string}, MuiButton.propTypes);
Button.defaultProps = _objectSpread({other: ''}, MuiButton.defaultProps);

Since the output yields both propTypes and defaultProps, it breaks tree shaking, and as a result,Button components ends up always bundling. (Because two or more property assignments on an object prevent its dead code removal in terser).

Any chance the destructuring syntax could be optimized? Thanks a lot in advance.

oliviertassinari commented 5 years ago

@alexnez96 Why are you using this approach in the first place? I can't think of a case that justify it.

The prop types and default props are considered private in Material-UI. We are moving from defaultProps to prop destructuring default value in v4.x.

ghost commented 5 years ago

The idea is since we're spreading the props on the MUI button props => <MuiButton {...props} />, it makes sense to inherit the prop types. But I guess if you don't, MuiButton will report the mismatch anyway, if any. I just thought it's better to re-declare them at the component as well (so as not to leak MUI implementation), since its props are own props + MUI props.

oliviertassinari commented 5 years ago

@alexnez96 You are right, it's not needed. You can save time by not doing it, the underlying element will warn if something is wrong :).

guigueb commented 3 years ago

I know this is an old and closed post but...

If, as we are, documentation for your class is generated from propTypes and defaultPropTypes... you do want to include information from inherited classes if you pass props through.