search

elidoran / cosmos-browserify

Browserify npm modules for client side in Meteor packages
MIT License
78 stars 12 forks source link

Added envify transform #5

Closed lourd closed 9 years ago

lourd commented 9 years ago

Sparked by https://github.com/grovelabs/meteor-react/issues/11, adding in an envify transform to the browserify build process enables dead code to be eliminated in production versions of some libraries like React. envify replaces the process.env calls with the string values set in the running process, and then UglifyJS, which Meteor runs automatically, eliminates any sections that are dead as a result.

I'm not aware of any downsides by replacing all instances of process.env.XXX unless there happened to be a malicious library that called something like process.env.AWS_SECRET_KEY and that got inserted and exposed on the client. That gets tricky if there's a nested dependency doing something fishy. But I don't think that'll be a problem.

elidoran commented 9 years ago

Thank you for this PR. I'm going to merge this and add some more to it.

elidoran commented 9 years ago

@lourd I used envify/custom so it accepts the environment value as an option and supplied it based on calculations it was already doing for the debug value.

I also altered the testing to check the envify stuff.

I also tried it in my cosmos-browserify-example project.

Does this still do what you wanted it to?

I added the purge option in there too, anyone see an issue with that?

lourd commented 9 years ago

That looks great! Just checking what I'm reading, NODE_ENV will be given as development unless it's meteor build or meteor bundle, then it's production. Unless the --debug flag was also specified, in which case it will still be development. Right?

Code's looking clean overall, nice work

elidoran commented 9 years ago

You read it correctly.

Thank you.

I've published this as 0.3.0.

mhagmajer commented 8 years ago

@elidoran We're using this Meteor package to include redux package on browser side and we found out that package.env.NODE_ENV references aren't replaced inside the contents of required packages. This results in a production runtime warning: https://github.com/rackt/redux/blob/master/src/index.js#L15 . They seem to only get replaced inside files directly referenced directly from package.js inside a Meteor package.

Is this a known issue? Are there any additional options we can pass to browserifiy to mitigate this problem?

CC @marcinsk