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

Wrapping propTypes with an environment check instead of removing them #35

Closed
insin opened this issue Jul 26, 2016 · 5 comments
Closed

Comments

@insin
Copy link
Contributor

insin commented Jul 26, 2016

Would it be feasible to wrap propType definitions in a if (process.env.NODE_ENV !== 'production') check (extracting them out first if they're part of a React.createClass()) instead of removing them completely?

This plugin is great and solves the problem of removing my own redundant propTypes for production, but it would be cool if it could also be adopted by authors transpiling reusable components to publish to npm, so your dependencies' propTypes could be available in development and test as normal, but automatically removed for production via the usual dead-code elimination process.

Could that be made an option into this plugin? Remove vs. move/wrap?

@insin insin changed the title Moving propTypes and wrapping them with an environment check Wrapping propTypes with an environment check instead of removing them Jul 26, 2016
@oliviertassinari
Copy link
Owner

oliviertassinari commented Jul 26, 2016

but it would be cool if its could also be adopted by authors transpiling reusable components to publish to npm

That sounds like a very good idea!
E.g. on one of my project, I'm letting Babel going through the source files of https://github.com/callemall/material-ui so I can remove the propTypes.

@oliviertassinari
Copy link
Owner

I'm working on it, it shouldn't be too hard, I'm already doing it here https://github.com/oliviertassinari/babel-plugin-transform-dev-warning/blob/master/src/index.js#L23.

@oliviertassinari
Copy link
Owner

@insin I have done a first iteration for this feature. I still miss the stage-1 class static syntax.

@insin
Copy link
Contributor Author

insin commented Jul 28, 2016

Amazing, thanks!

Looking forward to integrating this into my React component build setup 👍

@oliviertassinari
Copy link
Owner

oliviertassinari commented Jul 28, 2016

@insin I'm gonna add it to Material-UI too 🎉 .
However, I haven't tested it on a real project yet.
⚠️ It's still experimental.

I have two unsupported cases:

  • The old React.createClass.
  • The unnamed class.
    That should only be edges cases that we could support with more effort.

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

No branches or pull requests

2 participants