search

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

propTypes from stateless functional components not being removed #140

Open nijk opened 6 years ago

nijk commented 6 years ago

I'm aware that issues have been raised for this previously and work done to fix it.

However I'm seeing issues with a library (ReactPopper) that uses this plugin. The library currently uses this plugin but with { mode: 'wrap' } which results in minified builds that reference process.env - which is not something we want, react-popper GH Issue here. I've forked the library and made the config change to babel-plugin-transform-react-remove-prop-types here but I'm seeing that propTypes still exist on the stateless functional components:

Steps to reproduce

  1. git clone https://github.com/nijk/react-popper.git
  2. git checkout v0.x
  3. cd react-popper
  4. npm install
  5. grep propType dist/react-popper.min.js
  6. Observe that propTypes exists in the grep output, namely on the Target & Arrow components - both of which are stateless functional components.
oliviertassinari commented 6 years ago

@nijk Thanks for the reproduction. It sounds like a bug.

TrySound commented 6 years ago

Typescript output is also not processed. It contains propTypes inside of class iife but this plugins handles only top level propTypes assignments.

cocacrave commented 5 years ago

I'm experiencing same thing. At first I thought the plugin was not removing any propTypes but after reading this issue and looking into the build I noticed it was only the functional component's propTypes not being removed i.e. plugin removed propTypes from class components only.

oliviertassinari commented 5 years ago

I'm closing, the react popper issue is too high level. It doesn't mean it's an issue with this Babel plugin. Library authors should use the wrap mode. They should have the prop types in the modules they publish to npm. For a UMD release, this depends on the dev vs prod target.

nijk commented 5 years ago

@oliviertassinari Thanks, I've spent a few minutes generating a minimal reproduction of the issue here.

I may well be doing something wrong but here's what I found:

  1. When using the remove mode (default) the propTypes were not removed from either SFC or the ClassComponent.
  2. When using the wrap mode, the propTypes are wrapped with process.env - this is a problem for 3rd party React libs that can't really expect process.env to be available in the consuming app.
oliviertassinari commented 5 years ago
  1. When using the remove mode (default) the propTypes were not removed from either SFC or the ClassComponent.

We should fix that: https://github.com/nijk/babel-plugin-transform-react-remove-prop-types-issue-140.

  1. When using the wrap mode, the propTypes are wrapped with process.env - this is a problem for 3rd party React libs that can't really expect process.env to be available in the consuming app.

This is a no problem: https://unpkg.com/react.

nijk commented 5 years ago

@oliviertassinari is that not the development version of React? The production version doesn't attempt to access process.env.

nijk commented 5 years ago

Sorry, ignore that - I've just re-read the file you linked to

kylemh commented 5 years ago

Anything I can do to help push this along? I can't tell if this is a fix or not...