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

assets:compile doesn't respect NODE_ENV #1395

Closed
y310 opened this issue Apr 1, 2018 · 38 comments
Closed

assets:compile doesn't respect NODE_ENV #1395

y310 opened this issue Apr 1, 2018 · 38 comments

Comments

@y310
Copy link

y310 commented Apr 1, 2018

It seems that NODE_ENV=development bundle exec rails assets:precompile ignores NODE_ENV environment variable.

In order to confirm the problem, I added simple debug code to config/webpack/production.js

+if (process.env.NODE_ENV === 'production') {
+  throw 'NODE_ENV: ' + process.env.NODE_ENV;
+}
 process.env.NODE_ENV = process.env.NODE_ENV || 'production'

And here is the result.

/Users/mito/work/webpacker-test% NODE_ENV=development bundle exec rails assets:precompile
yarn install v1.5.1
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 1.25s.
Webpacker is installed 🎉 🍰
Using /Users/mito/work/webpacker-test/config/webpacker.yml file for setting up webpack paths
Compiling…
Compilation failed:


/Users/mito/work/webpacker-test/config/webpack/production.js:2
  throw 'NODE_ENV: ' + process.env.NODE_ENV;
  ^
NODE_ENV: production

In spite of NODE_ENV=development, process.env.NODE_ENV is production.
Probably, it's caused by this line https://github.com/rails/webpacker/blob/master/lib/tasks/webpacker/compile.rake#L24

This is a repository which includes the reproducing code.
https://github.com/y310/webpacker-test

@gauravtiwari
Copy link
Member

@y310 Thanks, ahh yes. The idea was to align with Sprockets assets:precompile so webpacker:compile always compiles in production mode. You could use NODE_ENV=development ./bin/webpack

Anyway would be good to know your use case?

@y310
Copy link
Author

y310 commented Apr 3, 2018

My case is a bit irregular... I run assets:precompile in CI to stabilize poltergeist and since my config/webpack/production.js relies on real production env, it throws an exception (I expected that config/webpack/development.js is executed but not the case)

If assets:precompile is made only for production env, I'd like to find other solutions for my problem.

@y310
Copy link
Author

y310 commented Apr 10, 2018

I'm going to close this issue since this behavior seems like a spec rather than bug.

@y310 y310 closed this as completed Apr 10, 2018
@ernsheong
Copy link

I want to run NODE_ENV=staging and use that to point my bundle to a different API endpoint used in staging vs production. That is my use case, and it seems odd that assets:precompile does not honour NODE_ENV.

@rubendinho
Copy link
Contributor

@gauravtiwari - We have spent hours debugging the above bug, since it directly contradicts the Custom Rails Environments instructions for this gem, which state that NODE_ENV can force assets:compile to compile in development mode:

# compiles in production mode by default unless NODE_ENV is specified
bundle exec rails assets:precompile
bundle exec rails webpacker:compile

If one has to use assets:precompile, is it possible to compile in development mode?

@csalvato
Copy link

csalvato commented Jan 9, 2019

Here's our use case:

We have a bug in one of our projects that is only appearing in production, but not in our dev environments. It likely has to do with the order of imports/compilation, and we wanted to debug that.

When debugging that, we wanted to compare a dev compile to a prod compile. We wound up going in circles just trying to figure out that NODE_ENV was not respected, and then trying to figure out why it wasn't (since the docs here say it should be:

https://github.com/rails/webpacker#custom-rails-environments

🤔

Running NODE_ENV=development ./bin/webpack throws an error:

$ NODE_ENV=development ./bin/webpack                                                                              1 ↵
module.js:491
    throw err;
    ^

Error: Cannot find module 'eslint-friendly-formatter'
    at Function.Module._resolveFilename (module.js:489:15)
    at Function.Module._load (module.js:439:25)
    at Module.require (module.js:517:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/username/path/to/project/config/webpack/loaders/eslint.js:8:16)
    at Module._compile (module.js:573:30)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)

So now we would have to debug or workaround that and the rabbit hole deepens...

@gauravtiwari gauravtiwari reopened this Jan 9, 2019
@gauravtiwari
Copy link
Member

Let's address this one properly, the idea earlier was to align with sprockets behaviour but seems like it's much more complicated in Webpacker's case.

@csalvato What happens if you run yarn install? I guess since yarn install is being run in production mode it's stripping off all the dev dependencies. Btw, what environment is this - CI?

@jakeNiemiec
Copy link
Member

@csalvato Can you double check that eslint-friendly-formatter is not under devDependancies in your package.json file? If so, try to move it to dependancies or make sure it is not include in production code (at require (internal/module.js:11:18)). More discussion of that in #1212

seems like it's much more complicated in Webpacker's case

I don't think webpack will ever fit nicely in a sprokets-sized hole. There will always be friction from those who expected webpack flows and those who expected sprokets flows...it's all about listening to user expectations. In this case, I don't think users expect (or want) webpackER to ignore the system-set NODE_ENV.

@csalvato
Copy link

csalvato commented Jan 9, 2019

@csalvato What happens if you run yarn install? I guess since yarn install is being run in production mode it's stripping off all the dev dependencies. Btw, what environment is this - CI?

@gauravtiwari This is the output:

$ yarn install
yarn install v1.10.0
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
warning "@rails/webpacker > [email protected]" has unmet peer dependency "caniuse-lite@^1.0.30000697".
warning " > [email protected]" has incorrect peer dependency "jquery@^1.7.0".
warning " > [email protected]" has unmet peer dependency "webpack@^2.0.0 || ^3.0.0".
warning "pdfjs-dist > [email protected]" has unmet peer dependency "webpack@^2.0.0 || ^3.0.0 || ^4.0.0".
warning " > [email protected]" has unmet peer dependency "webpack@^2.0.0 || >= 3.0.0-rc.0 || ^3.0.0".
warning " > [email protected]" has unmet peer dependency "css-loader@*".
warning " > [email protected]" has unmet peer dependency "webpack@^1.0.0 || ^2.0.0 || ^3.0.0".
warning "karma-webpack > [email protected]" has unmet peer dependency "webpack@^1.0.0 || ^2.0.0 || ^3.0.0".
warning " > [email protected]" has unmet peer dependency "webpack@^2.2.0 || ^3.0.0".
[5/5] 📃  Building fresh packages...
✨  Done in 31.92s.

Btw, what environment is this - CI?

Local dev environment. Not CI.

@csalvato Can you double check that eslint-friendly-formatter is not under devDependancies in your package.json file? If so, try to move it to dependancies or make sure it is not include in production code (at require (internal/module.js:11:18)). More discussion of that in #1212

Moving it to dependencies enabled NODE_ENV=development ./bin/webpack to work, and compiled the dev version 👍

@jakeNiemiec
Copy link
Member

@gauravtiwari I feel like this is a sufficiently reoccurring misconception that may justify a small warning in the README.md.

I have created a PR to add a small section about this to docs/yarn.md: #1880

@quinn
Copy link

quinn commented Feb 12, 2019

Yeah, this is very misleading in the docs, if you run:

RAILS_ENV=staging bundle exec rails assets:precompile

This will load config/webpack/production.js

There is no way to assets:precompile against anything other than config/webpack/production.js because it is hard-coded to 'production' in https://github.com/rails/webpacker/blob/master/lib/tasks/webpacker/compile.rake#L22

@bobziuchkovski
Copy link

Any progress on this? At the very least, I think the Custom Rails environments part of the docs should be updated to reflect reality.

I also wasted time trying to understand the difference in behavior between what the docs clearly state and what the gem is actually doing.

@jakeNiemiec
Copy link
Member

From the docs @bobziuchkovski linked:

Webpacker will use production environment as a fallback environment for loading configurations. Please note, NODE_ENV can either be set to production, development or test.

staging in not a valid NODE_ENV, so it falls back to production.

@quinn or @bobziuchkovski, can you tell me how we can make the docs less misleading?

@bobziuchkovski
Copy link

bobziuchkovski commented Jul 9, 2019

@jakeNiemiec Also from the docs:

And, this will compile in development mode and load configuration for cucumber environment if defined in webpacker.yml or fallback to production configuration

RAILS_ENV=cucumber NODE_ENV=development bundle exec rails assets:precompile

This isn't true. Running bundle exec rails assets:precompile is going to ignore NODE_ENV and always use NODE_ENV=production.

The "Please note, NODE_ENV can either be set to production, development or test" and the example of specifying NODE_ENV=development and running the asset precompile lead you to believe specifying NODE_ENV does something. However, with the hardcoded production NODE_ENV in the webpacker:compile rake task, this isn't the case.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Jul 9, 2019

I can see what you mean. It was probably correct up until d8dd446.

@bobziuchkovski
Copy link

Yes, I believe that's the case. My own feeling is that explicitly-provided ENV vars should take precedence over defaults, and that there seems to be some friction in the current implementation regarding RAILS_ENV -> NODE_ENV mapping.

My "magic wand" / ideal scenario would be:

  • Don't set anything for NODE_ENV in bin/webpack and bin/webpack-dev-server (no NODE_ENV ||= "development").
  • Support a node_env config param in config/webpacker.yml. Since this config file supports RAILS_ENV-specific values, it would allow clean mapping of RAILS_ENV -> NODE_ENV for folks who need it, or a simple default for folks that don't.
  • When NODE_ENV is explicitly specified in the ENV, use that explicitly-provided value. Otherwise use the value specified in config/webpacker.yml. By giving the ENV var the highest precedence, end-users have full control over the NODE_ENV that is utilized for bin/webpack, bin/webpack-dev-server, rake assets:precompile, and rake webpacker:compile. They can structure their workflow to match what the gem currently is doing or they can change it to better suit their needs.

I'm also okay accepting my opinion on the above might not be the consensus, but if that's the case, it would be nice to reflect reality in the docs. Since the docs called out being able to specify a NODE_ENV on the command line, I burned some amount of time trying to figure out why settings I placed in config/webpack/development.js weren't doing anything with NODE_ENV=development rake assets:precompile.

@jakeNiemiec
Copy link
Member

Sidenote: The most important thing to note: RAILS_ENV !== NODE_ENV. They do completely different, independent things and have very specific impacts on vanilla webpack, node, and yarn. They also have different valid options. The webpack bins seem to always respect NODE_ENV.

But none of that matters since (as of webpack@4) you can now set ./bin/webpack --mode=development. This should override anything webpackER tries to override.

From the webpack docs:

Option Description
development Sets process.env.NODE_ENV on DefinePlugin to value development. Enables NamedChunksPlugin and NamedModulesPlugin.
production Sets process.env.NODE_ENV on DefinePlugin to value production. Enables FlagDependencyUsagePlugin, FlagIncludedChunksPlugin, ModuleConcatenationPlugin, NoEmitOnErrorsPlugin, OccurrenceOrderPlugin, SideEffectsFlagPlugin and TerserPlugin.
none Opts out of any default optimization options

If not set, webpack sets production as the default value for mode.

Please remember that setting NODE_ENV doesn't automatically set mode.

I burned some amount of time trying to figure out why settings I placed in config/webpack/development.js weren't doing anything with NODE_ENV=development rake assets:precompile.

As stated above, the rake assets:compile tasks are intended to emulate Sprokets behavior.

PRs to address concerns are appreciated.

@bobziuchkovski
Copy link

bobziuchkovski commented Jul 10, 2019

PRs to address concerns are appreciated.

PRs to address documentation concerns or PRs to address the behavioral concerns? I'll gladly submit a PR to change the NODE_ENV-handling behavior, but it doesn't sound like either you or @gauravtiwari see the current behavior as problematic? I'd prefer to pursue behavioral changes only if I think there's a chance my changes will be accepted.

As stated above, the rake assets:compile tasks are intended to emulate Sprokets behavior.

Do you mean always using NODE_ENV=production for webpacker:compile is intended to emulate Sprockets behavior? If so, it doesn't seem like this achieves the intended goal. Does running assets:precompile change any of the Sprockets config as a result of running that task? As far as I'm aware, assets:precompile fully respects the application-configured Rails.application.config.assets.* params, which often differ between RAILS_ENV. In fact, generating a new rails project scaffolds different Rails.application.config.assets.* values into different config/environment/* files by default. It seems to me forcing NODE_ENV=production by means of running the rake task actually does the opposite of emulating Sprockets behavior.

@dblarons
Copy link

I'm inlining a client side API key with webpack / node-config (option 1 here) when I build my Docker image. The key must be different in QA / Prod environments.

But since NODE_ENV=qa isn't respected, I get the prod key in both environments. It would be nice if webpacker recognized more environments than prod/dev.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Sep 3, 2019

Assuming that you are still unable to use production, this should work better:

-NODE_ENV=qa
+NODE_ENV=test

In general, I would avoid baking API keys into your packs. I would use a file.js.erb to set the global value.

@dblarons
Copy link

dblarons commented Sep 3, 2019

Using NODE_ENV=test for a QA environment could have unintended effects in libraries that assume test means unit tests.

@dreamalligator
Copy link

cc @ixti

@coding-red-panda
Copy link
Contributor

Running into this as well now since we configure production only stuff in production.js and all this blows up the CI now.

@quinn
Copy link

quinn commented Oct 23, 2019

@coding-bunny I've adjusted my process to have NODE_ENV to only ever be equal to "development" or "production", and to only be used for whether the bundle is in "development mode", or for the case of prod or CI, the minified / bundled version

@coding-red-panda
Copy link
Contributor

That doesn't help us.
As I said, we have the config/webpack/production.js that always gets loaded now because of this issue, blowing up our CI, since we do not have access or the required environment variables present there that are needed. Nor do we even want to do this.

It also makes 0 sense that under a test environment, production code is loaded/executed.

@quinn
Copy link

quinn commented Oct 23, 2019

@coding-bunny it works out well for us, we can build a bundle once for CI, then deploy that same bundle to production (assuming it passes CI)

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Oct 23, 2019

since we do not have access or the required environment variables present there that are needed...It makes 0 sense that under a test environment, production code is loaded/executed.

@coding-bunny I'm under the impression that this is a major part of "integration testing"; to test the files you are sending to production as @quinn described. Perhaps you can give some more details on what you mean by "blowing up our CI"?

@coding-red-panda
Copy link
Contributor

When our applications are running in production mode, they need to call specific endpoints to register themselves. Only in production mode, because for the other environments, we simply don't care.

With this issue from webpacker, this causes our tests in CI to simply crash because either the endpoint they need to call is not available/reachable, or they don't have the required environment variables available to make these calls.

Why? Because webpacker decides to load production.js and the production configuration because of this issue, something that's not wanted at all when running explicitly in a testing environment.

So right now we have the following choices:

  • Don't use webpacker whatsoever ever
  • Put a lot of if statements in our production configuration files to detect whether we're running under a CI environment and skip sections
  • Refactor the entire sections of our code that perform this to not be loaded by webpacker.

Right now we're seriously going with option one if this is the behavior webpacker will be keeping.

@jakeNiemiec
Copy link
Member

Right now we're seriously going with option one if this is the behavior webpacker will be keeping.

Sprockets still works great.

When our applications are running in production mode, they need to call specific endpoints to register themselves... this causes our tests in CI to simply crash because either the endpoint they need to call is not available/reachable

This would usually be achieved mocking your endpoints. The script that sets up the mocks would conditionally load based on RAILS_ENV, this would be before any webpacker pack is loaded. Here is what Netflix uses to test their webpack bundles: https://github.com/Netflix/pollyjs

None of this is to say that this is the "right way" of doing things, you could even do something quick and dirty like:

<script>
  window.env = '<%= Rails&.env || nil %>' || 'unknown';

  window.isDev = function() {
    return window['env'] === 'development' || false;
  };
  window.isTest = function() {
    return window['env'] !== 'test';
  };
  window.isQa = function() {
    return window['env'] !== 'qa';
  };
  window.isProd = function() {
    return window['env'] !== 'development';
  };
</script>

Just put that script before before any webpacker pack is loaded. This way, the notion of NODE_ENV is completely discarded for RAILS_ENV.

@coding-red-panda
Copy link
Contributor

I'm not even getting to the part of running the tests because of this behavior!
bundle exec rails assets:precompile crashes because initializers that shouldn't be invoked are crashing due the problems described above. Same with bundle exec rails webpacker:precompile.

The only thing that works to even get into the testing phase is running bin/webpacker which allows us to bypass this entire mess.

@jakeNiemiec
Copy link
Member

If the problem happens at the initializer level, not the webpack level, can't the initializers check against RAILS_ENV instead of NODE_ENV? Would you mind opening another issue with the error message and other details?

@dblarons
Copy link

Hey @jakeNiemiec, what's the high-level rationale for restricting NODE_ENV to only production/development rather than respecting the NODE_ENV passed by the developer?

@coding-red-panda
Copy link
Contributor

@dblarons from what I understand they're trying to "emulate sprockets". A Framework that's actually configured to respect ENV inputs and works in/with different environments. As is the Rails convention.....

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Oct 30, 2019

@dblarons @coding-bunny Please realize that both rails & node.js have been around for a long time. Both have established practices. Don't expect NODE_ENV to follow the rules that RAILS_ENV follows.

high-level: Webpacker is a wrapper around the JS tool webpack. Webpack only accepts production or development, therefore webpackER should only accept production or development.

lower-level: Restricting NODE_ENVshields rails devs from edge cases that even afflict node.js devs that know what they're doing: https://glebbahmutov.com/blog/do-not-use-node-env-for-staging/

Hope that helps.

@coding-red-panda
Copy link
Contributor

No it doesn't help, because this is not even what's described in your README:

Out of the box Webpacker ships with - development, test and production environments in config/webpacker.yml however, in most production apps extra environments are needed as part of deployment workflow. Webpacker supports this out of the box from version 3.4.0+ onwards.

What's the point of having custom environments when it just boilds down loading either production.js or development.js?

@jakeNiemiec
Copy link
Member

What's the point of having custom environments when it just boilds down loading either production.js or development.js?

This was addressed here.

this is not even what's described in your README

Perhaps you could create a PR; that way, we can work together to fix discrepancies that you see.

I see that the reason this issue was reopened has been resolved, I am going t close this one since we have veered off topic. Feel free to create a new PR/issue with specific examples and I'm sure we can figure something out. (I don't want to spam the other folks subscribed)

@rails rails locked as resolved and limited conversation to collaborators Oct 30, 2019
@rails rails unlocked this conversation Nov 7, 2019
@jakeNiemiec
Copy link
Member

🎉 A solution by @coding-bunny has landed in #2341.

@dblarons
Copy link

dblarons commented Nov 7, 2019

🙏 thank you @coding-bunny !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests