Compile is always using production env?

[cmcfadden]

It seems like with the changes introduced in d8dd446, the webpack gem is always going to build in the production environment, regardless of the environment that's set via the environment variable.

Was this change intentional? Or am I missing something in terms of how to build non-production with this gem?

[gauravtiwari]

@cmcfadden Are you using the latest version? https://github.com/rails/webpacker/blob/master/lib/webpacker.rb#L16 & https://github.com/rails/webpacker/blob/3-x-stable/lib/webpacker.rb#L16 (3.5.3)

[cmcfadden]

We're using 3.5.3. The change to with_node_env you point to doesn't seem to matter in this case because the calling line is https://github.com/rails/webpacker/blob/master/lib/tasks/webpacker/compile.rake#L24 which hardcodes Webpacker.with_node_env("production") do

[davekerber]

Any update on this? It seems odd that you can only use the production environment to compile assets.

Edit: I'm happy to do a merge request for this. I just want to make sure there isn't a reason we don't want to use the specified environment.

[gauravtiwari]

@davekerber Sorry about the delay. The idea behind this was to have the same behavior as we have in assets:precompile for rails assets. I think that task always compiles in the production mode regardless of environment?

But I see the point, maybe assets:precompile should still run webpacker:compile in production mode however independently it should respect the environment provided. Would that work?

[davekerber]

@gauravtiwari also sorry for the delay. :) I agree, it should default to production, and should allow you to specify the environment. We regularly have staging or demo environments that have different settings from production. Should it be based on RAILS_ENV or NODE_ENV?

[dbalatero]

@gauravtiwari Yeah this is killing me too. I'm trying to precompile a test build on my CI server so that the parallel test runners can just pick up and run with the build. If I don't precompile, and just let Webpacker compile on the first integration spec that happens, it throws off the spec timings, which makes it impossible to load balance my test runners based on test time (as random tests will always take spec time + webpack compile time).

I'd like to prebuild with $ RAILS_ENV=test bundle exec rake assets:precompile, but my JS code has checks like if (process.env.RAILS_ENV === 'production') which get turned on erroneously in CI and break the build.

It doesn't make a lot of sense to me to force it to production in all instances. Alternatively another path to generate packs-test would be fine too? But this seems like the correct task to run to get what we need.

As mentioned above, if rake webpacker:compile respected whatever RAILS_ENV was set on the command line, I think this would be good. And keep assets:precompile forced to production.

Also as a suggestion, just put a line of output to assets:precompile that explicitly says that it's building in production mode. I spent 4 hours tracking down this behavior (I know it's in the docs but you have to find the right ones). Having a simple line of output might help future people debug this. You could say "If you want to prebuild a development/test bundle, use rake webpacker:compile" or something to that effect.

[dbalatero]

@davekerber happy to help with a PR if you've got too much on your plate!

[dbalatero]

For the time being, I added a custom Rake task to my Rails project to unblock me until this issue gets closed. Here's the patch if anyone else wants it:

# lib/tasks/webpacker.rake

namespace :ci do
  namespace :webpacker do
    task :compile do
      require 'webpacker'

      # Until https://github.com/rails/webpacker/issues/1558
      # is resolved, patch Webpacker to allow custom NODE_ENV in builds.
      module AllowNodeEnvInWebpackerCompile
        def with_node_env(*)
          puts "Compiling pack with NODE_ENV=#{ENV['NODE_ENV'] || 'production'}"
          yield
        end
      end

      Webpacker.extend(AllowNodeEnvInWebpackerCompile)

      Rake::Task['webpacker:compile'].invoke
    end
  end
end

[shepmaster]

This appears to continue to be a problem with webpacker 4.0.2. Our workaround is to do two separate precompilations:

RAILS_ENV=test bundle exec rake assets:precompile
RAILS_ENV=test bundle exec rake webpacker:compile

Only the second invocation creates the expected files in public/packs-test. Unfortunately, this increases our build times by 2 minutes (from 1:24 to 3:21).

[gauravtiwari]

Don't know but I just pasted the command in an example Rails 5 app with latest webpacker:

Compiles in expected directory.

[dbalatero]

@shepmaster @gauravtiwari

In my anecdotal usage, RAILS_ENV gets passed through correctly, but NODE_ENV is not, as with_node_env("production") clobbers whatever ENV["NODE_ENV"] is set to. I just submitted #2020 to attempt to fix this.

[gauravtiwari]

Yes, of course. Could you please tell us your use case? As explained in different threads, setting NODE_ENV to something other than development or production can have side effects too.

For example: You are deploying to staging environment and setting NODE_ENV to staging would build react in development mode, since it knows nothing about staging. Similarly, many other libraries have optimisations for either environment, therefore, I am not sure if this is a correct fix. Ideally, one shouldn't worry about NODE_ENV at all, just use RAILS_ENV to change settings or use other ENV variables.

That's why we kept it like so.

[dbalatero]

Yes, of course. Could you please tell us your use case?

I want to precompile my JS on my CI server with RAILS_ENV=test and NODE_ENV=development, as those are the correct envs for my testing environment.

I wouldn't deviate from NODE_ENV being development or production, as official Node docs warn away from it. When I need to detect things like whether I'm in test mode, I check process.env.RAILS_ENV === "test"

Edit: not precompiling my JS means that each of my parallel test runners has to spend 3 minutes compiling, as well as time pulling in build caches.

[shepmaster]

@gauravtiwari thank you for that output; I should have tested with a fresh Rails app 😊. Mine is indeed different (I've removed some unrelated warnings and the long list of assets):

$ RAILS_ENV=test bundle exec rake assets:precompile
cd "project" && RAILS_ENV=production bin/webpack
project/node_modules/.bin/webpack
--config
project/config/webpack/development.js
Hash: e7b645f5e0af9443c73a
Version: webpack 4.29.6
Time: 20236ms
Built at: 03/19/2019 1:40:12 PM
                                                                                 Asset       Size       Chunks             Chunk Names
                                                js/application-1480d778fe6ec7c80429.js   6.27 MiB  application  [emitted]  application
                                            js/application-1480d778fe6ec7c80429.js.map   5.26 MiB  application  [emitted]  application
                                                                         manifest.json   6.19 KiB               [emitted]
                                        media/assets/images/icons/many-more-images.svg   1.38 KiB               [emitted]

Entrypoint application = js/application-1480d778fe6ec7c80429.js js/application-1480d778fe6ec7c80429.js.map
[./app/javascript/bundles/SomeBundle/index.jsx] 1.44 KiB {application} [built]
    + 1440 hidden modules
yarn install v1.15.2
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
warning " > [email protected]" has incorrect peer dependency "redux@^3.0.0".
[5/5] 🔨  Building fresh packages...
✨  Done in 3.97s.

I'll have to investigate what the difference could be; my current hunch is some poor interaction with React on Rails.

@dbalatero describes well the end goal here — we have multiple parallel CI jobs. Running the first test with on-demand compilation actually causes our first test to fail due to long compilation times. We precompile the test assets once to:

• avoid compiling it once per parallel instance
• avoid the timeout on the first build

[jakeNiemiec]

causes our first test to fail due to long compilation times

How long is "long"? You might just need some optimization, how many packs are you packing?

[shepmaster]

my current hunch is some poor interaction with React on Rails.

I've opened shakacode/react_on_rails#1202; it does appear that RoR monkeys with webpackers tasks; apologies for the confusion!

@jakeNiemiec

How long is "long"?

I forget the exact numbers now, but that's a bit off topic for this issue. You can see my earlier comment where I mention that the precompilation takes 2 minutes.

We are lazy at the moment and only have one massive pack. It's not ideal, but we've made that tradeoff for now and aren't eager to change that yet.

Either way, performing precompilation once before the parallel test machines spin up saves us actual money by not wasting CPU time.

[gauravtiwari]

I want to precompile my JS on my CI server with RAILS_ENV=test and NODE_ENV=development, as those are the correct envs for my testing environment

I would use some other env variable for this, which would be more reliable vs depending on NODE_ENV. So you could just use RAILS_ENV=test bundle exec rails assets:precompile, which would load correct settings and then use different env variables for env specific code. For example: API_URL, AWS_KEY etc.

[dbalatero]

That's certainly a workaround but it's still not my preference. Consider that:

• external libraries take cues from NODE_ENV to strip out helpful development only messaging or utils that are useful in dev/test
• cursory googling shows that JS testing tools don't really seem to run in production mode although I could be totally wrong here
• it doesn't really make sense to block the developer from controlling their own build process in this way - it's easy enough to have a default of production and make it also overridable
• it's not intuitive that building a test pack would set node env to production. I spent hours chasing this down, and it seems like it violates principle of least surprise. Just like one wouldn't assume a RAILS_ENV of production when running their rspec suite.

If there's a desire to keep the compile task focused on production builds, I can buy that - but I don't really buy that setting node env to production is the safest assumption when wanting to build a precompiled test or dev pack, given the Node ecosystem doesn't necessarily work that way across all libs.

In any case the patch I have above "works" but I'd rather not patch things if it's possible to avoid.

[jakeNiemiec]

I forget the exact numbers now, but that's a bit off topic for this issue.

performing precompilation once before the parallel test machines spin up saves us actual money by not wasting CPU time.

I only asked because you said your tests would "fail due to long compilation times". Depending on what you use for a CI, there are tricks you can use to save your webpack cache (node_modules/.cache) to use in future builds. This is really handy if you compile a bunch of SCSS.

---

cursory googling shows that JS testing tools don't really seem to run in production mode although I could be totally wrong here

it's not intuitive that building a test pack would set node env to production. I spent hours chasing this down, and it seems like it violates principle of least surprise.

I personally would use production packs on a CI, but in general, TDD/unit-testing would seem to prefer dev or test, while e2e/integration testing would prefer test or production packs. I agree that NODE_ENV should be flexible as this would benefit both current and future tools.

On the other side of that coin, there have been users who follow the docs and install their testing framework like e.g:

yarn add --dev karma

They may find that their devDependencies are not installed because NODE_ENV was set to productionlike in: #1212 .

---

It just really depends on seeing what works best for the tools & tests you are using. At the end of the day, there does not seem to be one unifying opinion, even within testing libs:

• Setting NODE_ENV to test has unintended side effects jestjs/jest#3370 (comment)
• Would be nice if mocha sets the NODE_ENV variable to 'test' mochajs/mocha#185
• Feature request: set NODE_ENV=test avajs/ava#1470

[rossta]

Thanks for the lively discussion. This issue was fixed in #2341