Make "webpacker:yarn_install" task env aware #1331
Conversation
|
It'd be simpler if we can leave
NODE_ENV=development yarn install --silent && ls node_modules | wc -l
# 966
NODE_ENV=production yarn install --silent && ls node_modules | wc -l
# 666 |
|
Yes, lets remove production flag |
|
@gauravtiwari sure, I can update the PR. I'm assuming we'll want to set ENV["NODE_ENV"] ||= ENV["RAILS_ENV"]Let me know if this doesn't match your expectation |
|
@odlp Thank you :), no |
When webpacker compiles in a non-production environment it's removing node_modules which are dev dependencies. This is problematic on CI for example because these dependencies may be required.
This was failing on CI presumably because BUNDLE_GEMFILE was being lost, causing the 4.2.x Gemfile tests to fail.
If the NODE_ENV is set then Yarn will behave accordingly (e.g. skip devDependencies if NODE_ENV=production). Based on discussion: #1331 (comment)
|
@gauravtiwari PR updated. I ended up removing the If it's still required we can remove the last commit from the PR and still be in a state where Yarn install is environment aware (e.g. install |
test/test_helper.rb
Outdated
| @@ -30,4 +30,14 @@ def with_rails_env(env) | |||
| Rails.env = ActiveSupport::StringInquirer.new(original) | |||
| reloaded_config | |||
| end | |||
|
|
|||
| def with_environment(replacement_env) | |||
There was a problem hiding this comment.
I think this method and with_rails_env is the same. Could you please reuse it?
There was a problem hiding this comment.
I just tried using with_rails_env("test") and with_rails_env("production") but the tests fail:
Failure:
RakeTasksTest#test_rake_webpacker_yarn_install_in_production_environment [/Users/oli/workspace/webpacker/test/rake_tasks_test.rb:45]:
Expected only production dependencies to be installed.
Expected ["right-pad", "left-pad"] to not include "right-pad".
The Rake task uses system, which inherits environment variables from ENV I believe (hence the with_environment helper). system isn't aware of what's set in Rails.env.
There was a problem hiding this comment.
Ahh I see, my bad. Please try:
Webpacker.with_node_env("test") do
# block
endThere was a problem hiding this comment.
Ah nice, thanks for the pointer
|
Thanks @odlp nearly there, just one last tweak :) |
* Make webpacker:yarn_install task env aware When webpacker compiles in a non-production environment it's removing node_modules which are dev dependencies. This is problematic on CI for example because these dependencies may be required. * Use alternative to Bundler.with_clean_env This was failing on CI presumably because BUNDLE_GEMFILE was being lost, causing the 4.2.x Gemfile tests to fail. * Remove explicit production flag from Yarn Install task If the NODE_ENV is set then Yarn will behave accordingly (e.g. skip devDependencies if NODE_ENV=production). Based on discussion: rails/webpacker#1331 (comment) * Use existing with_node_env helper
* Make webpacker:yarn_install task env aware When webpacker compiles in a non-production environment it's removing node_modules which are dev dependencies. This is problematic on CI for example because these dependencies may be required. * Use alternative to Bundler.with_clean_env This was failing on CI presumably because BUNDLE_GEMFILE was being lost, causing the 4.2.x Gemfile tests to fail. * Remove explicit production flag from Yarn Install task If the NODE_ENV is set then Yarn will behave accordingly (e.g. skip devDependencies if NODE_ENV=production). Based on discussion: rails/webpacker#1331 (comment) * Use existing with_node_env helper
Addressing #1330, currently when webpacker compiles in a non-production environment it's removing
node_moduleswhich are dev dependencies.This is problematic on CI for example because these dependencies may be required to run JS unit tests and re-running
yarn installis inefficient.