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

Feature/webpack pr #46

Merged
merged 2 commits into from
Feb 13, 2018
Merged

Conversation

z-vr
Copy link
Contributor

@z-vr z-vr commented Feb 9, 2018

This allows to setup a dev environment with live-reload powered by webpack-dev-server.

I've added style.css as well so that people can do import 'react-popup/style.css' to get a default style.

@z-vr z-vr force-pushed the feature/webpack-pr branch 5 times, most recently from a1e1007 to d61a845 Compare February 10, 2018 21:56
@z-vr z-vr force-pushed the feature/webpack-pr branch from d61a845 to f0abab6 Compare February 10, 2018 22:07
@tbleckert tbleckert added this to the v1.0.0 milestone Feb 12, 2018
@tbleckert
Copy link
Contributor

Thanks! I've wanted to switch to webpack but haven't had the time so this is very much appreciated. I noticed you also moved the proptypes and defaultprops outside the class, is there a specific reason for this?

Great idea to add the example and style.css parts as well.

I'm really grateful for your help!

@z-vr
Copy link
Contributor Author

z-vr commented Feb 12, 2018

@tbleckert the reason is basically to enable stripping PropTypes for production mode with this plugin https://github.com/oliviertassinari/babel-plugin-transform-react-remove-prop-types, but I'm not sure if it's that useful, also I don't know what happens if components rely on default props, e.g., calling a stub () => {} function

@tbleckert
Copy link
Contributor

Oh nice! Should be fine I think, either way it will be tested properly before it goes out to npm. I'll merge it in.

@tbleckert tbleckert merged commit aa185fc into minutemailer:master Feb 13, 2018
@z-vr z-vr deleted the feature/webpack-pr branch February 27, 2018 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants