Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make compilation stale on asset host change #364

Merged
merged 19 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ _Please add entries here for your pull requests that are not yet released._

### 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)
- Recompile assets on asset host change [PR 364](https://github.com/shakacode/shakapacker/pull/364) by [ahangarha](https://github.com/ahangarha).

## [v7.1.0] - September 30, 2023

Expand Down
4 changes: 3 additions & 1 deletion lib/shakapacker/digest_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def watched_files_digest
end
files = Dir[*expanded_paths].reject { |f| File.directory?(f) }
file_ids = files.sort.map { |f| "#{File.basename(f)}/#{Digest::SHA1.file(f).hexdigest}" }
Digest::SHA1.hexdigest(file_ids.join("/"))

asset_host = Shakapacker.config.asset_host.to_s
Digest::SHA1.hexdigest(file_ids.join("/").concat(asset_host))
ahangarha marked this conversation as resolved.
Show resolved Hide resolved
end

def record_compilation_digest
Expand Down
1 change: 1 addition & 0 deletions spec/backward_compatibility_specs/digest_strategy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ def remove_compilation_digest_path

before :all do
@digest_strategy = Webpacker::DigestStrategy.new
ENV["SHAKAPACKER_ASSET_HOST"] = nil
ahangarha marked this conversation as resolved.
Show resolved Hide resolved
remove_compilation_digest_path
end

Expand Down
14 changes: 5 additions & 9 deletions spec/shakapacker/compiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@
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

custom_env_variables = {
"SHAKAPACKER_ASSET_HOST" => "foo.bar",
"SHAKAPACKER_RELATIVE_URL_ROOT" => "/baz"
}

with_env_variable(custom_env_variables) do
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
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
22 changes: 10 additions & 12 deletions spec/shakapacker/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -341,17 +341,16 @@
end

it "returns the value of SHAKAPACKER_ASSET_HOST if set" do
with_env_variable("SHAKAPACKER_ASSET_HOST" => "custom_host.abc") do
expect(config.asset_host).to eq "custom_host.abc"
end
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")

with_env_variable("SHAKAPACKER_ASSET_HOST" => nil) do
expect(config.asset_host).to eq "domain.abc"
end
expect(config.asset_host).to eq "domain.abc"
end
end

Expand All @@ -365,17 +364,16 @@
end

it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do
with_env_variable("SHAKAPACKER_RELATIVE_URL_ROOT" => "custom_value") do
expect(config.relative_url_root).to eq "custom_value"
end
allow(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.helpers.compute_asset_host 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")

with_env_variable("SHAKAPACKER_RELATIVE_URL_ROOT" => nil) do
expect(config.relative_url_root).to eq "abcd"
end
expect(config.relative_url_root).to eq "abcd"
end
end
end
22 changes: 22 additions & 0 deletions spec/shakapacker/digest_strategy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,29 @@ def remove_compilation_digest_path
expect(@digest_strategy.fresh?).to be true
end

it "is stale when host changes" do
allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("old-host")
# Record the digests for old-host
@digest_strategy.after_compile_hook

allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("new-host")
expect(@digest_strategy.stale?).to be true
expect(@digest_strategy.fresh?).to be_falsey
end

it "generates correct compilation_digest_path" do
allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("custom-path")

actual_path = @digest_strategy.send(:compilation_digest_path).basename.to_s
host_hash = Digest::SHA1.hexdigest("custom-path")
expected_path = "last-compilation-digest-#{Shakapacker.env}"

expect(actual_path).to eq expected_path
end

it "generates correct compilation_digest_path without the digest of the asset host if asset host is not set" do
allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil)

actual_path = @digest_strategy.send(:compilation_digest_path).basename.to_s
expected_path = "last-compilation-digest-#{Shakapacker.env}"

Expand Down
18 changes: 0 additions & 18 deletions spec/shakapacker/spec_helper_initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,3 @@ def with_rails_env(env)
Rails.env = ActiveSupport::StringInquirer.new(original)
reloaded_config
end

# Temportarily set env variables to a custom value
# arg: a hash with key:value for each custom env
# Keys could be string or symbol
def with_env_variable(custom_env_hash)
original_env = {}
custom_env_hash.each do |key, new_value|
upcased_key = key.to_s.upcase
original_env[upcased_key] = new_value
ENV[upcased_key] = new_value
end

yield
ensure
original_env.each do |key, original_value|
ENV[key] = original_value
end
end