Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

propTypes from stateless functional components not being removed #140

Open
nijk opened this issue Apr 19, 2018 · 9 comments
Open

propTypes from stateless functional components not being removed #140

nijk opened this issue Apr 19, 2018 · 9 comments
Labels

Comments

@nijk
Copy link

nijk commented Apr 19, 2018

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
Copy link
Owner

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

@TrySound
Copy link

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

@cocacrave
Copy link

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
Copy link
Owner

oliviertassinari commented Jan 15, 2019

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
Copy link
Author

nijk commented Jan 15, 2019

@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
Copy link
Owner

oliviertassinari commented Jan 15, 2019

  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
Copy link
Author

nijk commented Jan 15, 2019

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

@nijk
Copy link
Author

nijk commented Jan 15, 2019

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

@kylemh
Copy link

kylemh commented Jul 16, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants