Fix usage of RAILS_ENV #366
Conversation
| @@ -2,7 +2,7 @@ module Shakapacker | |||
| class Runner | |||
| def self.run(argv) | |||
| $stdout.sync = true | |||
| ENV["NODE_ENV"] ||= (ENV["RAILS_ENV"] == "production") ? "production" : "development" | |||
| ENV["NODE_ENV"] ||= ENV["RAILS_ENV"] || "development" | |||
There was a problem hiding this comment.
What if you have a RAILS_ENV of staging or something else.
The only fully supported value of NODE_ENV is 'production'.
Otherewise, we want to use 'development'.
There was a problem hiding this comment.
The problem with supporting only the production or development here is that in some cases, you may have some assets that may need to be compiled in against test environment.
We, at decidim had a very strange issue, when our compiling was ok, which meant that assets were successfuly built, but in the same time, when running bundle exec rspec with no assets, the pipeline failed due to different compiling strategy between prod and test.
Please see a more detailed explanation the bug #365.
Additionally you may want to see the decidim/decidim#11728, in which, in order to fully fix the issue explained, i have ended up in having the TerserPlugin enabled on all my environments.
I will shortly post a comment on #365 explaining how i got into this mess, and maybe the proposed fix, does not to be applied :)
There was a problem hiding this comment.
The comment was added #365 (comment)
There was a problem hiding this comment.
@alecslupu if you want a custom NODE_ENV, then why not just set it before running the scripts that invoke this command?
| @@ -3,7 +3,7 @@ $stdout.sync = true | |||
| namespace :shakapacker do | |||
| desc "Compile JavaScript packs using webpack for production with digests" | |||
| task compile: ["shakapacker:verify_install", :environment] do | |||
| Shakapacker.with_node_env(ENV.fetch("NODE_ENV", "production")) do | |||
| Shakapacker.with_node_env(ENV.fetch("NODE_ENV", ENV["RAILS_ENV"] || "production")) do | |||
| @@ -21,7 +21,7 @@ def load_config | |||
| @config = Configuration.new( | |||
| root_path: app_root, | |||
| config_path: Pathname.new(@shakapacker_config), | |||
| env: ENV["RAILS_ENV"] | |||
| env: ENV["RAILS_ENV"] || "development" | |||
There was a problem hiding this comment.
Why would there be no RAILS_ENV?
This is probably ok, as maybe it's weird to run the the dev_server_runner outside of development?
The part of the shakapacker.yml used is based on the RAILS_ENV.
https://github.com/shakacode/shakapacker/blob/master/lib/shakapacker/configuration.rb#L130
|
Waiting for @alecslupu. |
|
@alecslupu Please let me know. If I don't hear from you, I'll close, and we can consider re-opening. |
|
We can close this. As previously said, the issue was the fact that prod and test environment had different compiling strategies, that impacted the libraries i was used. |
Summary
This PR attempts to properly match NODE_ENV to RAILS_ENV in case of NODE_ENV not being set.
Pull Request checklist
Remove this line after checking all the items here. If the item does not apply to the PR, both check it out and wrap it by
~.Other Information
Fixes: #365