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

replace undefined environment variables with undefined #16

Closed
wants to merge 3 commits into from

Conversation

tjconcept
Copy link

fixes #11

@hughsk
Copy link
Owner

hughsk commented Nov 11, 2014

Thanks for the patch @tjconcept! I'm curious though, what's the benefit in replacing these values with undefined? They already evaluate to undefined at runtime, even without envify:

$ echo 'console.log(process.env.NODE_ENV)' | browserify - | node
undefined

Additionally, there are some cases where you don't want envify to touch environment variables that haven't been set, leaving them for a second envify transform step to take care of.

@tjconcept
Copy link
Author

I think it's cleaner, and I can avoid stubbing the process variable all together.
I hadn't thought of the "double use" use case though..

@hughsk
Copy link
Owner

hughsk commented Nov 12, 2014

@tjconcept I don't think you need to worry about the cleanliness of your code once it's been through browserify :) If you want to explicitly remove the variable to avoid all of the process shim getting pulled in, you should be able to do:

browserify -t [ envify --no-NODE_ENV ]

Which will replace process.env.NODE_ENV with false. Would that fix your issue?

@tjconcept
Copy link
Author

Yes it would, but I would also have to specify 4-5 other variables.
My use case is client applications which communicate with several web services and 95% of the time the build just goes with the default values, but it's necessary to configure each of these in a production environment or while cross-developing a feature in a service and in the client.

Maybe this could be added as optional? I still think it is the "expected" behavior though, as @Naman34 in #11 did.

@hughsk hughsk closed this in 05506ab Nov 12, 2014
@hughsk
Copy link
Owner

hughsk commented Nov 12, 2014

@tjconcept, @Naman34, I've gone ahead and implemented this as an optional feature. You should be able to use it by adding purge as an argument to envify:

browserify -t [ envify purge --NODE_ENV development ]

More details in the readme :)

@nmn
Copy link

nmn commented Nov 13, 2014

Thanks!

This pull request was closed.
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.

undefined values?
3 participants