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

Remove react prop-types for production build (fix) #112

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

spartDev
Copy link
Contributor

Since react-media 1.9.0 and adding the __DEV__ boolean global, there is a problem in the final production build.
When importing the lib in your project an error occurred :

ReferenceError: __DEV__ is not defined

I use babel-plugin-transform-react-remove-prop-types to remove react prop-typesin production instead of using the __DEV__ global.

@spartDev spartDev changed the title remove react prop-types for production build Remove react prop-types for production build (fix) Dec 12, 2018
@edorivai
Copy link
Collaborator

Thanks for the quick PR. I'm verifying some stuff on my end, and plan to roll this patch out today!

@edorivai edorivai self-assigned this Dec 12, 2018
@edorivai edorivai merged commit 5201239 into ReactTraining:master Dec 12, 2018
@mjackson
Copy link
Member

The thing that was nice about using the __DEV__ flag is that we can still include prop-types in development but remove them in production. Also, __DEV__ works for more than just prop-types; we can use it anywhere we want to put development-only code.

The way this PR is implemented, people using our ESM build will never get prop-types warnings, even in development where they could be useful.

It looks like it wasn't working because I forgot to include babel-plugin-dev-expression in our babel config. I'll go ahead and revert this commit and fix it another way.

Thanks for the catch, @spartDev :)

@spartDev
Copy link
Contributor Author

@mjackson I understand about using __DEV__ flag but babel-plugin-transform-react-remove-prop-types remove import reference too.
What about create an esmconfig for production?

@mjackson
Copy link
Member

The import reference will be removed by webpack in production mode if it isn't used anywhere in our code. We don't need an esm build for production because ESM imports are unconditional, so everyone imports the same build and then uses process.env.NODE_ENV (same as in React) to create their own production build.

@spartDev
Copy link
Contributor Author

@mjackson fair point!

mjackson added a commit that referenced this pull request Dec 12, 2018
This reverts commit 5201239, reversing
changes made to 18a87a3.
mjackson added a commit that referenced this pull request Dec 12, 2018
This will automatically do the right thing with our __DEV__ statements
that we use to prune out development-only code from our production
builds.

Helps #111
Fixes #112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants