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

prop-types source is included in dist builds #943

Closed
nkbt opened this issue May 3, 2018 · 8 comments
Closed

prop-types source is included in dist builds #943

nkbt opened this issue May 3, 2018 · 8 comments

Comments

@nkbt
Copy link

nkbt commented May 3, 2018

It would be nice to have source of prop-types removed from dist. Especially minified version

This helps: https://www.npmjs.com/package/babel-plugin-transform-react-remove-prop-types

In non-minified build node_modules/react-redux/dist/react-redux.js

image

In minified build node_modules/react-redux/dist/react-redux.min.js

image

@gaearon
Copy link
Contributor

gaearon commented May 3, 2018

@timdorr
Copy link
Member

timdorr commented May 3, 2018

I'm cool with that. I'll check into it for the next release.

@nkbt
Copy link
Author

nkbt commented May 7, 2018

Unfortunately babel-plugin-transform-react-remove-prop-types does not solve the problem, since it does not remove anything from contextTypes which react-redux uses (and issue is closed now as basically won't fix: oliviertassinari/babel-plugin-transform-react-remove-prop-types#77)

For my library I've done a little hack that works well, but I would be reluctant to recommend it to everyone:

  1. webpack config: https://github.com/in-flux/react-component-router/blob/master/scripts/utils/webpack/common.js#L89-L96
  2. actual file: https://github.com/in-flux/react-component-router/blob/master/scripts/utils/webpack/emptyPropTypes.js

@timdorr
Copy link
Member

timdorr commented Nov 6, 2018

This will be fixed once we do our next release.

@timdorr timdorr closed this as completed Nov 6, 2018
@artem-malko
Copy link

Hello @timdorr I know, this is quite old issue, but it should be reopened, as I think.
Yes, you've removed prop-types from react-redux.min.js, but prop types like:

Provider.propTypes = {
  store: PropTypes.shape({
    subscribe: PropTypes.func.isRequired,
    dispatch: PropTypes.func.isRequired,
    getState: PropTypes.func.isRequired
  }),
  context: PropTypes.object,
  children: PropTypes.any
};

are still in a file.
Moreover, than I build my app with webpack, all proptypes are in my bundle, cuase they are required in components/Provider.js. And terser or uglifyJs ca not strip it.

What do you think, if we just wrap prop types to NODE_ENV check like this:

if (process.env.NODE_ENV !== 'production') {
  Provider.propTypes = {
    store: PropTypes.shape({
      subscribe: PropTypes.func.isRequired,
      dispatch: PropTypes.func.isRequired,
      getState: PropTypes.func.isRequired
    }),
    context: PropTypes.object,
    children: PropTypes.any
  };
}

So, prop-types will be removed from prod build in that case.

@nkbt
Copy link
Author

nkbt commented Nov 16, 2019

@artem-malko there are ways to automatically remove them without polluting code. See my last comment with config examples. You can do it yourself for your prod app build

@artem-malko
Copy link

@nkbt ok, thx) But I can not understand, why do we need prop-types in prod code?

@nkbt
Copy link
Author

nkbt commented Nov 17, 2019

@artem-malko I know, right! After all that was me who opened the issue :)

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

No branches or pull requests

4 participants