From fc57a005a426c9f02e4c3efd7465550a5cfa69af Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 21:44:42 +0100 Subject: [PATCH 1/7] Revert d8dd4460144e63a9120290631c492aba3cbea332 Undoing the commit that forces the production environment. The code has it as default when not defined, this is sufficient, but it should respect the user settings! Resolves #2340 --- lib/tasks/webpacker/compile.rake | 18 +++++++++--------- lib/webpacker.rb | 8 -------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/lib/tasks/webpacker/compile.rake b/lib/tasks/webpacker/compile.rake index db5970bb7..2a0edf592 100644 --- a/lib/tasks/webpacker/compile.rake +++ b/lib/tasks/webpacker/compile.rake @@ -1,3 +1,5 @@ +ENV["NODE_ENV"] ||= "production" + $stdout.sync = true def ensure_log_goes_to_stdout @@ -26,15 +28,13 @@ end namespace :webpacker do desc "Compile JavaScript packs using webpack for production with digests" task compile: ["webpacker:verify_install", :environment] do - Webpacker.with_node_env("production") do - ensure_log_goes_to_stdout do - if Webpacker.compile - # Successful compilation! - else - # Failed compilation - exit! - end - end + ensure_log_goes_to_stdout do + if Webpacker.compile + # Successful compilation! + else + # Failed compilation + exit! + end end end end diff --git a/lib/webpacker.rb b/lib/webpacker.rb index 9dddb6939..87db03c4f 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -14,14 +14,6 @@ def instance @instance ||= Webpacker::Instance.new end - def with_node_env(env) - original = ENV["NODE_ENV"] - ENV["NODE_ENV"] = env - yield - ensure - ENV["NODE_ENV"] = original - end - delegate :logger, :logger=, :env, to: :instance delegate :config, :compiler, :manifest, :commands, :dev_server, to: :instance delegate :bootstrap, :clean, :clobber, :compile, to: :commands From 0cd8dfbbe34a173a74b89578cd3ec391cd0ef8fa Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 21:53:14 +0100 Subject: [PATCH 2/7] Update compile.rake use proper indentation --- lib/tasks/webpacker/compile.rake | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/tasks/webpacker/compile.rake b/lib/tasks/webpacker/compile.rake index 2a0edf592..56b8e29a1 100644 --- a/lib/tasks/webpacker/compile.rake +++ b/lib/tasks/webpacker/compile.rake @@ -29,12 +29,12 @@ namespace :webpacker do desc "Compile JavaScript packs using webpack for production with digests" task compile: ["webpacker:verify_install", :environment] do ensure_log_goes_to_stdout do - if Webpacker.compile - # Successful compilation! - else - # Failed compilation - exit! - end + if Webpacker.compile + # Successful compilation! + else + # Failed compilation + exit! + end end end end From 976fc9c04fa184ea7ea64f105cb9bcb46309f9b3 Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 21:57:39 +0100 Subject: [PATCH 3/7] Update rake_tasks_test.rb Removed call to `with_node_env` and assume production by default. --- test/rake_tasks_test.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/rake_tasks_test.rb b/test/rake_tasks_test.rb index eb6e547bd..1f01c6893 100644 --- a/test/rake_tasks_test.rb +++ b/test/rake_tasks_test.rb @@ -42,10 +42,8 @@ def test_rake_webpacker_yarn_install_in_non_production_environments end def test_rake_webpacker_yarn_install_in_production_environment - Webpacker.with_node_env("production") do - Dir.chdir(test_app_path) do - `bundle exec rake webpacker:yarn_install` - end + Dir.chdir(test_app_path) do + `bundle exec rake webpacker:yarn_install` end refute_includes installed_node_module_names, "right-pad", From 293263bb39752a4038de588ebdf677694a9b54fc Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 22:01:13 +0100 Subject: [PATCH 4/7] Update rake_tasks_test.rb updated failing tests to reflect changes made. --- test/rake_tasks_test.rb | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/test/rake_tasks_test.rb b/test/rake_tasks_test.rb index 1f01c6893..903aeccb3 100644 --- a/test/rake_tasks_test.rb +++ b/test/rake_tasks_test.rb @@ -28,20 +28,18 @@ def test_rake_task_webpacker_check_binstubs refute_includes output, "webpack binstubs not found." end - def test_rake_webpacker_yarn_install_in_non_production_environments - assert_includes test_app_dev_dependencies, "right-pad" - - Webpacker.with_node_env("test") do - Dir.chdir(test_app_path) do - `bundle exec rake webpacker:yarn_install` - end + def test_rake_webpacker_yarn_install_in_default_environment + Dir.chdir(test_app_path) do + `bundle exec rake webpacker:yarn_install` end - assert_includes installed_node_module_names, "right-pad", - "Expected dev dependencies to be installed" + refute_includes installed_node_module_names, "right-pad", + "Expected only production dependencies to be installed" end - - def test_rake_webpacker_yarn_install_in_production_environment + + def test_rake_webpacker_yarn_install_in_explicit_production_environment + ENV["NODE_ENV"] = "production" + Dir.chdir(test_app_path) do `bundle exec rake webpacker:yarn_install` end @@ -49,6 +47,17 @@ def test_rake_webpacker_yarn_install_in_production_environment refute_includes installed_node_module_names, "right-pad", "Expected only production dependencies to be installed" end + + def test_rake_webpacker_yarn_install_in_non_production_environment + ENV["NODE_ENV"] = "test" + + Dir.chdir(test_app_path) do + `bundle exec rake webpacker:yarn_install` + end + + assert_includes installed_node_module_names, "right-pad", + "Expected development dependencies to be installed" + end private def test_app_path From efd4225fed3ce749dd5ee8186310add10742a551 Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 22:05:48 +0100 Subject: [PATCH 5/7] Update rake_tasks_test.rb inject the ENV over command line --- test/rake_tasks_test.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/rake_tasks_test.rb b/test/rake_tasks_test.rb index 903aeccb3..9fd5743a2 100644 --- a/test/rake_tasks_test.rb +++ b/test/rake_tasks_test.rb @@ -38,10 +38,8 @@ def test_rake_webpacker_yarn_install_in_default_environment end def test_rake_webpacker_yarn_install_in_explicit_production_environment - ENV["NODE_ENV"] = "production" - Dir.chdir(test_app_path) do - `bundle exec rake webpacker:yarn_install` + `NODE_ENV=production bundle exec rake webpacker:yarn_install` end refute_includes installed_node_module_names, "right-pad", @@ -49,10 +47,8 @@ def test_rake_webpacker_yarn_install_in_explicit_production_environment end def test_rake_webpacker_yarn_install_in_non_production_environment - ENV["NODE_ENV"] = "test" - Dir.chdir(test_app_path) do - `bundle exec rake webpacker:yarn_install` + `NODE_ENV=test bundle exec rake webpacker:yarn_install` end assert_includes installed_node_module_names, "right-pad", From 6974048fc946443b45d973331157ef14bb02dcb3 Mon Sep 17 00:00:00 2001 From: coding-bunny Date: Wed, 30 Oct 2019 22:09:22 +0100 Subject: [PATCH 6/7] Update rake_tasks_test.rb trying to fix RuboCop --- test/rake_tasks_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/rake_tasks_test.rb b/test/rake_tasks_test.rb index 9fd5743a2..b80f43791 100644 --- a/test/rake_tasks_test.rb +++ b/test/rake_tasks_test.rb @@ -36,7 +36,7 @@ def test_rake_webpacker_yarn_install_in_default_environment refute_includes installed_node_module_names, "right-pad", "Expected only production dependencies to be installed" end - + def test_rake_webpacker_yarn_install_in_explicit_production_environment Dir.chdir(test_app_path) do `NODE_ENV=production bundle exec rake webpacker:yarn_install` @@ -45,7 +45,7 @@ def test_rake_webpacker_yarn_install_in_explicit_production_environment refute_includes installed_node_module_names, "right-pad", "Expected only production dependencies to be installed" end - + def test_rake_webpacker_yarn_install_in_non_production_environment Dir.chdir(test_app_path) do `NODE_ENV=test bundle exec rake webpacker:yarn_install` From 88be57fa9e0d9941cc85fb7d11fdecc30ad55621 Mon Sep 17 00:00:00 2001 From: Arne De Herdt Date: Thu, 7 Nov 2019 09:01:23 +0100 Subject: [PATCH 7/7] Using @rossta solution to combine both behaviors --- lib/tasks/webpacker/compile.rake | 16 ++++++++-------- lib/webpacker.rb | 8 ++++++++ test/rake_tasks_test.rb | 31 ++++++++++++++----------------- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/lib/tasks/webpacker/compile.rake b/lib/tasks/webpacker/compile.rake index 56b8e29a1..b8422cd41 100644 --- a/lib/tasks/webpacker/compile.rake +++ b/lib/tasks/webpacker/compile.rake @@ -1,5 +1,3 @@ -ENV["NODE_ENV"] ||= "production" - $stdout.sync = true def ensure_log_goes_to_stdout @@ -28,12 +26,14 @@ end namespace :webpacker do desc "Compile JavaScript packs using webpack for production with digests" task compile: ["webpacker:verify_install", :environment] do - ensure_log_goes_to_stdout do - if Webpacker.compile - # Successful compilation! - else - # Failed compilation - exit! + Webpacker.with_node_env(ENV.fetch("NODE_ENV", "production")) do + ensure_log_goes_to_stdout do + if Webpacker.compile + # Successful compilation! + else + # Failed compilation + exit! + end end end end diff --git a/lib/webpacker.rb b/lib/webpacker.rb index 87db03c4f..9dddb6939 100644 --- a/lib/webpacker.rb +++ b/lib/webpacker.rb @@ -14,6 +14,14 @@ def instance @instance ||= Webpacker::Instance.new end + def with_node_env(env) + original = ENV["NODE_ENV"] + ENV["NODE_ENV"] = env + yield + ensure + ENV["NODE_ENV"] = original + end + delegate :logger, :logger=, :env, to: :instance delegate :config, :compiler, :manifest, :commands, :dev_server, to: :instance delegate :bootstrap, :clean, :clobber, :compile, to: :commands diff --git a/test/rake_tasks_test.rb b/test/rake_tasks_test.rb index b80f43791..eb6e547bd 100644 --- a/test/rake_tasks_test.rb +++ b/test/rake_tasks_test.rb @@ -28,33 +28,30 @@ def test_rake_task_webpacker_check_binstubs refute_includes output, "webpack binstubs not found." end - def test_rake_webpacker_yarn_install_in_default_environment - Dir.chdir(test_app_path) do - `bundle exec rake webpacker:yarn_install` + def test_rake_webpacker_yarn_install_in_non_production_environments + assert_includes test_app_dev_dependencies, "right-pad" + + Webpacker.with_node_env("test") do + Dir.chdir(test_app_path) do + `bundle exec rake webpacker:yarn_install` + end end - refute_includes installed_node_module_names, "right-pad", - "Expected only production dependencies to be installed" + assert_includes installed_node_module_names, "right-pad", + "Expected dev dependencies to be installed" end - def test_rake_webpacker_yarn_install_in_explicit_production_environment - Dir.chdir(test_app_path) do - `NODE_ENV=production bundle exec rake webpacker:yarn_install` + def test_rake_webpacker_yarn_install_in_production_environment + Webpacker.with_node_env("production") do + Dir.chdir(test_app_path) do + `bundle exec rake webpacker:yarn_install` + end end refute_includes installed_node_module_names, "right-pad", "Expected only production dependencies to be installed" end - def test_rake_webpacker_yarn_install_in_non_production_environment - Dir.chdir(test_app_path) do - `NODE_ENV=test bundle exec rake webpacker:yarn_install` - end - - assert_includes installed_node_module_names, "right-pad", - "Expected development dependencies to be installed" - end - private def test_app_path File.expand_path("test_app", __dir__)