facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.47k stars 358 forks source link

browserify doesn't replace NODE_ENV #267

Open dwelle opened 5 years ago

dwelle commented 5 years ago

Unless I misunderstand, resulting bundle from browserify still contains some process.env.NODE_ENV references because the prop-types package depends on a another package that does not specify browserify.transform in its package.json. In particular, the react-is package (haven't checked if there are others).

npx browserify -r prop-types | grep NODE_ENV

The browserify.transform is a non-global transform, thus it doesn't transform dependencies.

ljharb commented 5 years ago

This is true; it's pretty important to apply envify at the app level. I'm not sure how else to address this issue.

The only other dep is object-assign and there shouldn't be anything env-specific in there.

dwelle commented 5 years ago

Yea, one way is for the user to run custom global transform like:

stream.transform(envify(), { global: true })

which is something the envify docs don't even mention (probably because transforming node_modules is not recommended, but necessary in this case).

Other solution is to go through all deps that react owns (such as react-is) and add the transform into package.json same as other react libs already do. But problem are deps that react doesn't own :/.

EDIT: closed by mistake -- keep/close as you will

ljharb commented 5 years ago

Typically bundles are where the distinction matters, and bundlers do typically replace it in the entire bundle - it's not much of a problem for node.

Perhaps you could file an issue on the react repo for react-is, and then this one can be closed?

oliviertassinari commented 5 months ago

It feels like removing the dependency on loose-envify would make sense. Referencing: https://github.com/facebook/react/issues/27748 where this is discussed.