Consistently use NODE_ENV and support non-standard env

[gauravtiwari]

Closes #1265

This PR fixes issue for non-standard environments

• Adds separate env class for gem
• Consistent gem and npm package behaviour
• Use NODE_ENV for consistency
• Removes deprecated set from ConfigList type
• Adds tests

cc// @rmehner @IgorDmitriev @doits @hiromi2424 do you wanna try out this branch please?

[javan]

We probably don't want to set any ENV vars here since this file gets required by applications. It was OK to do in bin/* files since those are separate one-off processes.

[gauravtiwari]

Ahh right, I was thinking setting here will set it in all environments (like in rails console) but I guess that's not ideal.

[javan]

Does this allow RAILS_ENV and NODE_ENV to be set separately / independently? I imagine there are cases where you want NODE_ENV to be production and RAILS_ENV to be something else. Like deploying your Rails app to a staging environment.

[gauravtiwari]

Yep it was possible before as well except that we were falling back to RAILS_ENV if there was no NODE_ENV set. I removed that case for clarity so it always checks for NODE_ENV first and then falls back to 'production'. What do you think?

[javan]

What happens when there's no config/webpack/[environment].js file matching the env? I can't tell at a glance.

[gauravtiwari]

So if the config file is missing then binstub will raise an error (same as Rails env config):

If the config file is present matching environment but the settings isn't present inside webpacker.yml for that environment:

If both config file and env is defined in webpacker.yml then:

[gauravtiwari]

The one thing I am unsure is:

If we open rails console without NODE_ENV and try to access Webpacker instance then it raises an error. I am not sure if we should use RAILS_ENV as fallback or don't raise at all? The reason for removing RAILS_ENV was to keep it consistent with JS ecosystem - so it's either NODE_ENV or production fallback to make things clear.

That's why I moved the env declaration to module itself from binstubs so NODE_ENV is consistently set but I guess that's isn't ideal?

[hiromi2424]

My Circle CI test was failed with following message:

Webpacker is installed 🎉 🍰
Using /******/config/webpacker.yml file for setting up webpack paths
rails aborted!
Using webpacker requires that you set NODE_ENV environment variables. Valid values are default, development, test, staging, production

We expect webpacker to fallback RAILS_ENV, that ours is 'test' 😢

If this is breaking change please update minor(or major) version..

[gauravtiwari]

@hiromi2424 Ahh I see, what is NODE_ENV set to test?

[hiromi2424]

@gauravtiwari It passed with NODE_ENV: test .

RAILS_ENV fallback also allow us to separate webpacker env and application env, e.g.:

LET

RAILS_ENV = staging
webpacker.yml contains:

• development
• test
• production

ACTS

It fallbacks production config for webpacker.
And when NODE_ENV=test specified, webpacker runs with test config, but rails side it runs on staging environment.
yay!

---

I think raising error is not comfortable decision since webpacker is for rails application that requires RAILS_ENV but not NODE_ENV.

[sunnyrjuneja]

I highly endorse the idea of using RAILS_ENV as a fallback for NODE_ENV.

[gauravtiwari]

alright makes sense, will push an update 👍

[hiromi2424]

Huge thanks! @gauravtiwari
Now CI and staging build of my project has been succeeded!!!

[Systho]

@gauravtiwari
We still can't use NODE_ENV=staging RAILS_ENV=staging (Or just RAILS_ENV=staging)?

What would be the expected way to provide a different webpacker config in custom environment ?

The typical use case would be using different JS API keys.

Currently I'm using different sections in webpacker.yml to set the values then the DefinePlugin like this in config/environment.js

environment.plugins.append('Define', new webpack.DefinePlugin({
  "GA_PROPERTY_ID":        JSON.stringify(config.ga_property_id),
  ....
}));

[gauravtiwari]

@Systho Sorry about that, I am working on a PR to fix this.

[Systho]

No problem, It's great that you're aware of the problem.
Since this issue is closed I was mostly wondering whether I was doing something wrong. That's why I provided my use case :)

Keep up the good work and thank you !