From 30f70c9197dd7a126161e9042e7622f508ef42b0 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Tue, 7 Nov 2023 12:01:05 +0330 Subject: [PATCH 01/11] Fix mistake in test title --- spec/shakapacker/configuration_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index f41e61bde..28f9cc001 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -369,7 +369,7 @@ expect(config.relative_url_root).to eq "custom_value" end - it "returns ActionController::Base.helpers.compute_asset_host if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do + it "returns ActionController::Base.relative_url_root if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do allow(ActionController::Base).to receive(:relative_url_root).and_return("abcd") allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd") From 5f4508237fc6e10c2ac28193c61b0a71f32ab7a8 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Tue, 7 Nov 2023 12:02:03 +0330 Subject: [PATCH 02/11] Ensure string result from methods --- lib/shakapacker/configuration.rb | 4 ++-- spec/backward_compatibility_specs/compiler_spec.rb | 4 ++-- spec/shakapacker/compiler_spec.rb | 4 ++-- spec/shakapacker/configuration_spec.rb | 12 ++++++++++++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index 033ef3bdb..5da78ca01 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -117,11 +117,11 @@ def fetch(key) end def asset_host - ENV.fetch("SHAKAPACKER_ASSET_HOST", ActionController::Base.helpers.compute_asset_host) + ENV.fetch("SHAKAPACKER_ASSET_HOST", ActionController::Base.helpers.compute_asset_host).to_s end def relative_url_root - ENV.fetch("SHAKAPACKER_RELATIVE_URL_ROOT", ActionController::Base.relative_url_root) + ENV.fetch("SHAKAPACKER_RELATIVE_URL_ROOT", ActionController::Base.relative_url_root).to_s end private diff --git a/spec/backward_compatibility_specs/compiler_spec.rb b/spec/backward_compatibility_specs/compiler_spec.rb index 9bdd98dce..7d80d17d0 100644 --- a/spec/backward_compatibility_specs/compiler_spec.rb +++ b/spec/backward_compatibility_specs/compiler_spec.rb @@ -47,8 +47,8 @@ end it "accepts external env variables" do - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to be nil - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to be nil + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" ENV["WEBPACKER_ASSET_HOST"] = "foo.bar" ENV["WEBPACKER_RELATIVE_URL_ROOT"] = "/baz" diff --git a/spec/shakapacker/compiler_spec.rb b/spec/shakapacker/compiler_spec.rb index 672a85e71..1fbd3b4ab 100644 --- a/spec/shakapacker/compiler_spec.rb +++ b/spec/shakapacker/compiler_spec.rb @@ -47,8 +47,8 @@ end it "accepts external env variables" do - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to be nil - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to be nil + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("foo.bar") allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("/baz") diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index 28f9cc001..91f18270c 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -340,6 +340,12 @@ ) end + it "returns an empty string if neither SHAKAPACKER_ASSET_HOST nor ActionController::Base.helpers.compute_asset_host is set" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil) + + expect(config.asset_host).to eq "" + end + it "returns the value of SHAKAPACKER_ASSET_HOST if set" do expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("custom_host.abc") @@ -363,6 +369,12 @@ ) end + it "returns an empty string if neither SHAKAPACKER_RELATIVE_URL_ROOT nor ActionController::Base.relative_url_root is set" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) + + expect(config.relative_url_root).to eq "" + end + it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") From f4329de4170ea7f4b51184caacb92e1ed1b84ae1 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Tue, 7 Nov 2023 17:31:54 +0330 Subject: [PATCH 03/11] Use asset_host in config file as the second priority --- lib/install/config/shakapacker.yml | 7 +++ lib/shakapacker/configuration.rb | 10 +++- spec/shakapacker/configuration_spec.rb | 69 ++++++++++++++++++-------- 3 files changed, 63 insertions(+), 23 deletions(-) diff --git a/lib/install/config/shakapacker.yml b/lib/install/config/shakapacker.yml index c071d1319..9d96cd342 100644 --- a/lib/install/config/shakapacker.yml +++ b/lib/install/config/shakapacker.yml @@ -50,6 +50,13 @@ default: &default # https://webpack.js.org/guides/build-performance/#avoid-production-specific-tooling useContentHash: false + # Setting the asset host here will override Rails.application.config.asset_host. + # Here, you can set different asset_host per environment. Note that + # SHAKAPACKER_ASSET_HOST will override both configurations. + # asset_host: custom-path + + # relative_url_root: custom-path + development: <<: *default compile: true diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index 5da78ca01..101aa8453 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -117,11 +117,17 @@ def fetch(key) end def asset_host - ENV.fetch("SHAKAPACKER_ASSET_HOST", ActionController::Base.helpers.compute_asset_host).to_s + ENV.fetch( + "SHAKAPACKER_ASSET_HOST", + fetch(:asset_host) || ActionController::Base.helpers.compute_asset_host + ).to_s end def relative_url_root - ENV.fetch("SHAKAPACKER_RELATIVE_URL_ROOT", ActionController::Base.relative_url_root).to_s + ENV.fetch( + "SHAKAPACKER_RELATIVE_URL_ROOT", + fetch(:relative_url_root) || ActionController::Base.relative_url_root + ).to_s end private diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index 91f18270c..0b454871f 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -340,23 +340,37 @@ ) end - it "returns an empty string if neither SHAKAPACKER_ASSET_HOST nor ActionController::Base.helpers.compute_asset_host is set" do - expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil) - - expect(config.asset_host).to eq "" - end - it "returns the value of SHAKAPACKER_ASSET_HOST if set" do expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("custom_host.abc") expect(config.asset_host).to eq "custom_host.abc" end - it "returns ActionController::Base.helpers.compute_asset_host if SHAKAPACKER_ASSET_HOST is not set" do - allow(ActionController::Base.helpers).to receive(:compute_asset_host).and_return("domain.abc") - allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", "domain.abc").and_return("domain.abc") + context "without SHAKAPACKER_ASSET_HOST set" do + it "returns asset_host in shakapacker.yml if set" do + expect(config).to receive(:fetch).with(:asset_host).and_return("value-in-config-file.com") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", "value-in-config-file.com").and_return("value-in-config-file.com") + + expect(config.asset_host).to eq "value-in-config-file.com" + end + + context "without asset_host set in the shakapacker.yml" do + it "returns ActionController::Base.helpers.compute_asset_host if SHAKAPACKER_ASSET_HOST is not set" do + expect(config).to receive(:fetch).with(:asset_host).and_return(nil) + expect(ActionController::Base.helpers).to receive(:compute_asset_host).and_return("domain.abc") + allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", "domain.abc").and_return("domain.abc") - expect(config.asset_host).to eq "domain.abc" + expect(config.asset_host).to eq "domain.abc" + end + + context "without ActionController::Base.helpers.compute_asset_host returing any value" do + it "returns an empty string" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil) + + expect(config.asset_host).to eq "" + end + end + end end end @@ -369,23 +383,36 @@ ) end - it "returns an empty string if neither SHAKAPACKER_RELATIVE_URL_ROOT nor ActionController::Base.relative_url_root is set" do - expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) - - expect(config.relative_url_root).to eq "" - end - it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do - allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") expect(config.relative_url_root).to eq "custom_value" end - it "returns ActionController::Base.relative_url_root if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do - allow(ActionController::Base).to receive(:relative_url_root).and_return("abcd") - allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd") + context "without SHAKAPACKER_RELATIVE_URL_ROOT set" do + it "returns relative_url_root in shakapacker.yml if set" do + expect(config).to receive(:fetch).with(:relative_url_root).and_return("value-in-config-file") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "value-in-config-file").and_return("value-in-config-file") + + expect(config.relative_url_root).to eq "value-in-config-file" + end + + context "without relative_url_root set in the shakapacker.yml" do + it "returns ActionController::Base.relative_url_root if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do + expect(ActionController::Base).to receive(:relative_url_root).and_return("abcd") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd") - expect(config.relative_url_root).to eq "abcd" + expect(config.relative_url_root).to eq "abcd" + end + + context "without ActionController::Base.relative_url_root returing any value" do + it "returns an empty string" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) + + expect(config.relative_url_root).to eq "" + end + end + end end end end From 7b8380047c13cb302ba1aec34f24c503a40c280e Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Tue, 7 Nov 2023 17:47:22 +0330 Subject: [PATCH 04/11] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce68ea28e..2c0d70dad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ _Please add entries here for your pull requests that are not yet released._ ### Added - Experimental support for other JS package managers using `package_json` gem [PR 349](https://github.com/shakacode/shakapacker/pull/349) by [G-Rath](https://github.com/g-rath). - Support `hmr: only` configuration [PR 378](https://github.com/shakacode/shakapacker/pull/378) by [SimenB](https://github.com/SimenB). +- Use `config/shakapacker.yml` and the secondary source for `asset_host` and `relative_url_root` configurations [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). ### Fixed - Recommend `server` option instead of deprecated `https` option when `--https` is provided [PR 380](https://github.com/shakacode/shakapacker/pull/380) by [G-Rath](https://github.com/g-rath) From 7e00d32264893c62a65d37483d47274a2d8d39c7 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Fri, 17 Nov 2023 13:38:08 +0330 Subject: [PATCH 05/11] Fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c0d70dad..b885f1740 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ _Please add entries here for your pull requests that are not yet released._ ### Added - Experimental support for other JS package managers using `package_json` gem [PR 349](https://github.com/shakacode/shakapacker/pull/349) by [G-Rath](https://github.com/g-rath). - Support `hmr: only` configuration [PR 378](https://github.com/shakacode/shakapacker/pull/378) by [SimenB](https://github.com/SimenB). -- Use `config/shakapacker.yml` and the secondary source for `asset_host` and `relative_url_root` configurations [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). +- Use `config/shakapacker.yml` as the secondary source for `asset_host` and `relative_url_root` configurations [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). ### Fixed - Recommend `server` option instead of deprecated `https` option when `--https` is provided [PR 380](https://github.com/shakacode/shakapacker/pull/380) by [G-Rath](https://github.com/g-rath) From 4a363efba9f53cbc7782fd1317e4e79c20061afe Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Sat, 18 Nov 2023 23:24:49 +0330 Subject: [PATCH 06/11] Remove any references to relative_url_root Despite being added in webpacker, it never got used. There is no functionality tied to this configuration and this commit, removes its references in the code and tests. --- CHANGELOG.md | 2 +- lib/install/config/shakapacker.yml | 2 - lib/shakapacker/compiler.rb | 1 - lib/shakapacker/configuration.rb | 7 ---- .../compiler_spec.rb | 3 -- spec/shakapacker/compiler_spec.rb | 3 -- spec/shakapacker/configuration_spec.rb | 42 ------------------- 7 files changed, 1 insertion(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b885f1740..6c7a29af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ _Please add entries here for your pull requests that are not yet released._ ### Added - Experimental support for other JS package managers using `package_json` gem [PR 349](https://github.com/shakacode/shakapacker/pull/349) by [G-Rath](https://github.com/g-rath). - Support `hmr: only` configuration [PR 378](https://github.com/shakacode/shakapacker/pull/378) by [SimenB](https://github.com/SimenB). -- Use `config/shakapacker.yml` as the secondary source for `asset_host` and `relative_url_root` configurations [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). +- Use `config/shakapacker.yml` as the secondary source for `asset_host` configuration [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). ### Fixed - Recommend `server` option instead of deprecated `https` option when `--https` is provided [PR 380](https://github.com/shakacode/shakapacker/pull/380) by [G-Rath](https://github.com/g-rath) diff --git a/lib/install/config/shakapacker.yml b/lib/install/config/shakapacker.yml index 9d96cd342..6a4df72e3 100644 --- a/lib/install/config/shakapacker.yml +++ b/lib/install/config/shakapacker.yml @@ -55,8 +55,6 @@ default: &default # SHAKAPACKER_ASSET_HOST will override both configurations. # asset_host: custom-path - # relative_url_root: custom-path - development: <<: *default compile: true diff --git a/lib/shakapacker/compiler.rb b/lib/shakapacker/compiler.rb index 4c0dcb094..ff54fe1a9 100644 --- a/lib/shakapacker/compiler.rb +++ b/lib/shakapacker/compiler.rb @@ -108,7 +108,6 @@ def webpack_env env.merge( "SHAKAPACKER_ASSET_HOST" => instance.config.asset_host, - "SHAKAPACKER_RELATIVE_URL_ROOT" => instance.config.relative_url_root, "SHAKAPACKER_CONFIG" => instance.config_path.to_s ) end diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index 101aa8453..e1824229f 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -123,13 +123,6 @@ def asset_host ).to_s end - def relative_url_root - ENV.fetch( - "SHAKAPACKER_RELATIVE_URL_ROOT", - fetch(:relative_url_root) || ActionController::Base.relative_url_root - ).to_s - end - private def data @data ||= load diff --git a/spec/backward_compatibility_specs/compiler_spec.rb b/spec/backward_compatibility_specs/compiler_spec.rb index 7d80d17d0..e5ec75a5b 100644 --- a/spec/backward_compatibility_specs/compiler_spec.rb +++ b/spec/backward_compatibility_specs/compiler_spec.rb @@ -48,12 +48,9 @@ it "accepts external env variables" do expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" ENV["WEBPACKER_ASSET_HOST"] = "foo.bar" - ENV["WEBPACKER_RELATIVE_URL_ROOT"] = "/baz" expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar" - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz" end end diff --git a/spec/shakapacker/compiler_spec.rb b/spec/shakapacker/compiler_spec.rb index 1fbd3b4ab..7fb7a4068 100644 --- a/spec/shakapacker/compiler_spec.rb +++ b/spec/shakapacker/compiler_spec.rb @@ -48,12 +48,9 @@ it "accepts external env variables" do expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("foo.bar") - allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("/baz") expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar" - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz" end end diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index 0b454871f..a2746383a 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -373,46 +373,4 @@ end end end - - describe "#relative_url_root" do - let(:config) do - Shakapacker::Configuration.new( - root_path: ROOT_PATH, - config_path: Pathname.new(File.expand_path("./test_app/config/shakapacker.yml", __dir__)), - env: "production" - ) - end - - it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do - expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") - - expect(config.relative_url_root).to eq "custom_value" - end - - context "without SHAKAPACKER_RELATIVE_URL_ROOT set" do - it "returns relative_url_root in shakapacker.yml if set" do - expect(config).to receive(:fetch).with(:relative_url_root).and_return("value-in-config-file") - expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "value-in-config-file").and_return("value-in-config-file") - - expect(config.relative_url_root).to eq "value-in-config-file" - end - - context "without relative_url_root set in the shakapacker.yml" do - it "returns ActionController::Base.relative_url_root if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do - expect(ActionController::Base).to receive(:relative_url_root).and_return("abcd") - expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd") - - expect(config.relative_url_root).to eq "abcd" - end - - context "without ActionController::Base.relative_url_root returing any value" do - it "returns an empty string" do - expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) - - expect(config.relative_url_root).to eq "" - end - end - end - end - end end From 4b32eaa991ce7b7dc6c921262460a6a7b4b85fc3 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Wed, 6 Dec 2023 13:19:05 +0330 Subject: [PATCH 07/11] Revert "Remove any references to relative_url_root" This reverts commit 2677f81290dabb4cbd8e24c2590706ba94f777ed. We will address it later. --- CHANGELOG.md | 2 +- lib/install/config/shakapacker.yml | 2 + lib/shakapacker/compiler.rb | 1 + lib/shakapacker/configuration.rb | 7 ++++ .../compiler_spec.rb | 3 ++ spec/shakapacker/compiler_spec.rb | 3 ++ spec/shakapacker/configuration_spec.rb | 42 +++++++++++++++++++ 7 files changed, 59 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c7a29af4..b885f1740 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ _Please add entries here for your pull requests that are not yet released._ ### Added - Experimental support for other JS package managers using `package_json` gem [PR 349](https://github.com/shakacode/shakapacker/pull/349) by [G-Rath](https://github.com/g-rath). - Support `hmr: only` configuration [PR 378](https://github.com/shakacode/shakapacker/pull/378) by [SimenB](https://github.com/SimenB). -- Use `config/shakapacker.yml` as the secondary source for `asset_host` configuration [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). +- Use `config/shakapacker.yml` as the secondary source for `asset_host` and `relative_url_root` configurations [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). ### Fixed - Recommend `server` option instead of deprecated `https` option when `--https` is provided [PR 380](https://github.com/shakacode/shakapacker/pull/380) by [G-Rath](https://github.com/g-rath) diff --git a/lib/install/config/shakapacker.yml b/lib/install/config/shakapacker.yml index 6a4df72e3..9d96cd342 100644 --- a/lib/install/config/shakapacker.yml +++ b/lib/install/config/shakapacker.yml @@ -55,6 +55,8 @@ default: &default # SHAKAPACKER_ASSET_HOST will override both configurations. # asset_host: custom-path + # relative_url_root: custom-path + development: <<: *default compile: true diff --git a/lib/shakapacker/compiler.rb b/lib/shakapacker/compiler.rb index ff54fe1a9..4c0dcb094 100644 --- a/lib/shakapacker/compiler.rb +++ b/lib/shakapacker/compiler.rb @@ -108,6 +108,7 @@ def webpack_env env.merge( "SHAKAPACKER_ASSET_HOST" => instance.config.asset_host, + "SHAKAPACKER_RELATIVE_URL_ROOT" => instance.config.relative_url_root, "SHAKAPACKER_CONFIG" => instance.config_path.to_s ) end diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index e1824229f..101aa8453 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -123,6 +123,13 @@ def asset_host ).to_s end + def relative_url_root + ENV.fetch( + "SHAKAPACKER_RELATIVE_URL_ROOT", + fetch(:relative_url_root) || ActionController::Base.relative_url_root + ).to_s + end + private def data @data ||= load diff --git a/spec/backward_compatibility_specs/compiler_spec.rb b/spec/backward_compatibility_specs/compiler_spec.rb index e5ec75a5b..7d80d17d0 100644 --- a/spec/backward_compatibility_specs/compiler_spec.rb +++ b/spec/backward_compatibility_specs/compiler_spec.rb @@ -48,9 +48,12 @@ it "accepts external env variables" do expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" ENV["WEBPACKER_ASSET_HOST"] = "foo.bar" + ENV["WEBPACKER_RELATIVE_URL_ROOT"] = "/baz" expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar" + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz" end end diff --git a/spec/shakapacker/compiler_spec.rb b/spec/shakapacker/compiler_spec.rb index 7fb7a4068..1fbd3b4ab 100644 --- a/spec/shakapacker/compiler_spec.rb +++ b/spec/shakapacker/compiler_spec.rb @@ -48,9 +48,12 @@ it "accepts external env variables" do expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("foo.bar") + allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("/baz") expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar" + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz" end end diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index a2746383a..0b454871f 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -373,4 +373,46 @@ end end end + + describe "#relative_url_root" do + let(:config) do + Shakapacker::Configuration.new( + root_path: ROOT_PATH, + config_path: Pathname.new(File.expand_path("./test_app/config/shakapacker.yml", __dir__)), + env: "production" + ) + end + + it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") + + expect(config.relative_url_root).to eq "custom_value" + end + + context "without SHAKAPACKER_RELATIVE_URL_ROOT set" do + it "returns relative_url_root in shakapacker.yml if set" do + expect(config).to receive(:fetch).with(:relative_url_root).and_return("value-in-config-file") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "value-in-config-file").and_return("value-in-config-file") + + expect(config.relative_url_root).to eq "value-in-config-file" + end + + context "without relative_url_root set in the shakapacker.yml" do + it "returns ActionController::Base.relative_url_root if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do + expect(ActionController::Base).to receive(:relative_url_root).and_return("abcd") + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd") + + expect(config.relative_url_root).to eq "abcd" + end + + context "without ActionController::Base.relative_url_root returing any value" do + it "returns an empty string" do + expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) + + expect(config.relative_url_root).to eq "" + end + end + end + end + end end From 3dcebb0ea7ef93aeb8025d0ec7857a40a96abd7e Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Thu, 7 Dec 2023 15:06:54 +0330 Subject: [PATCH 08/11] Remove relative_url_root from config file --- lib/install/config/shakapacker.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/install/config/shakapacker.yml b/lib/install/config/shakapacker.yml index 9d96cd342..6a4df72e3 100644 --- a/lib/install/config/shakapacker.yml +++ b/lib/install/config/shakapacker.yml @@ -55,8 +55,6 @@ default: &default # SHAKAPACKER_ASSET_HOST will override both configurations. # asset_host: custom-path - # relative_url_root: custom-path - development: <<: *default compile: true From 7374cb5d524967f910c9403697b1d864f4f07eec Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Thu, 7 Dec 2023 15:14:01 +0330 Subject: [PATCH 09/11] Show deprecation message for `relateive_url_root` --- lib/shakapacker/configuration.rb | 2 ++ spec/shakapacker/configuration_spec.rb | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index 101aa8453..8274da1cd 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -124,6 +124,8 @@ def asset_host end def relative_url_root + Shakapacker.puts_deprecation_message "The usage of relative_url_root is deprecated in Shakapacker and will be removed in v8." + ENV.fetch( "SHAKAPACKER_RELATIVE_URL_ROOT", fetch(:relative_url_root) || ActionController::Base.relative_url_root diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index 0b454871f..17c9ee643 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -383,6 +383,10 @@ ) end + it "shows deprecation message" do + expect { config.relative_url_root }.to output(/deprecated/).to_stdout + end + it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value") From dd6484e29ca5b2ad4fe48f652e35330ebb991999 Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Thu, 7 Dec 2023 15:17:02 +0330 Subject: [PATCH 10/11] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b885f1740..0dab3273a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ _Please add entries here for your pull requests that are not yet released._ - Recompile assets on asset host change [PR 364](https://github.com/shakacode/shakapacker/pull/364) by [ahangarha](https://github.com/ahangarha). - Add deprecation warning for `https` option in `shakapacker.yml` (use `server: 'https'` instead) [PR 382](https://github.com/shakacode/shakapacker/pull/382) by [G-Rath](https://github.com/g-rath). +### Deprecated +- The usage of relative_url_root is deprecated in Shakapacker and will be removed in v8. [PR 376](https://github.com/shakacode/shakapacker/pull/376) by [ahangarha](https://github.com/ahangarha). + ## [v7.1.0] - September 30, 2023 ### Added From 663f2943675f15b6ecdb810758873c976001eaee Mon Sep 17 00:00:00 2001 From: Mostafa Ahangarha Date: Thu, 21 Dec 2023 18:57:24 +0330 Subject: [PATCH 11/11] Revert changes and return nil for asset_host and relative_url_root if not provided --- lib/shakapacker/configuration.rb | 4 ++-- spec/backward_compatibility_specs/compiler_spec.rb | 4 ++-- spec/shakapacker/compiler_spec.rb | 4 ++-- spec/shakapacker/configuration_spec.rb | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/shakapacker/configuration.rb b/lib/shakapacker/configuration.rb index 8274da1cd..d702f8ed5 100644 --- a/lib/shakapacker/configuration.rb +++ b/lib/shakapacker/configuration.rb @@ -120,7 +120,7 @@ def asset_host ENV.fetch( "SHAKAPACKER_ASSET_HOST", fetch(:asset_host) || ActionController::Base.helpers.compute_asset_host - ).to_s + ) end def relative_url_root @@ -129,7 +129,7 @@ def relative_url_root ENV.fetch( "SHAKAPACKER_RELATIVE_URL_ROOT", fetch(:relative_url_root) || ActionController::Base.relative_url_root - ).to_s + ) end private diff --git a/spec/backward_compatibility_specs/compiler_spec.rb b/spec/backward_compatibility_specs/compiler_spec.rb index 7d80d17d0..9bdd98dce 100644 --- a/spec/backward_compatibility_specs/compiler_spec.rb +++ b/spec/backward_compatibility_specs/compiler_spec.rb @@ -47,8 +47,8 @@ end it "accepts external env variables" do - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" - expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to be nil + expect(Webpacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to be nil ENV["WEBPACKER_ASSET_HOST"] = "foo.bar" ENV["WEBPACKER_RELATIVE_URL_ROOT"] = "/baz" diff --git a/spec/shakapacker/compiler_spec.rb b/spec/shakapacker/compiler_spec.rb index 1fbd3b4ab..672a85e71 100644 --- a/spec/shakapacker/compiler_spec.rb +++ b/spec/shakapacker/compiler_spec.rb @@ -47,8 +47,8 @@ end it "accepts external env variables" do - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "" - expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "" + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to be nil + expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to be nil allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("foo.bar") allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("/baz") diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb index 17c9ee643..79cbc5443 100644 --- a/spec/shakapacker/configuration_spec.rb +++ b/spec/shakapacker/configuration_spec.rb @@ -364,10 +364,10 @@ end context "without ActionController::Base.helpers.compute_asset_host returing any value" do - it "returns an empty string" do + it "returns nil" do expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil) - expect(config.asset_host).to eq "" + expect(config.asset_host).to be nil end end end @@ -413,7 +413,7 @@ it "returns an empty string" do expect(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return(nil) - expect(config.relative_url_root).to eq "" + expect(config.relative_url_root).to be nil end end end