Skip to content

Commit

Permalink
Raise MissingAssetError in compute_asset_path instead of each resolver
Browse files Browse the repository at this point in the history
Follow-up to 0fd7814:
Rather than adding this `raise` to each resolver's `resolve` method,
let's move it into Propshaft's `compute_asset_path` implementation.
`compute_asset_path` is the ActionView method that asset pipeline gems
are expected to implement [1] [2], which makes it 1) the highest level
for us to catch missing assets and 2) the entry point for anyone looking
to understand how Propshaft integrates with the ActionView helpers
(thus, it's helpful to have that method's definition explicitly include
the exception it raises, just like sprockets-rails does [3]).

[1]: https://github.com/rails/rails/blob/099289b6360ac82d1e0fa0a5592ac10cfc05e6e0/actionview/lib/action_view/helpers/asset_url_helper.rb#L132-L133
[2]: https://github.com/rails/rails/blob/099289b6360ac82d1e0fa0a5592ac10cfc05e6e0/actionview/lib/action_view/helpers/asset_url_helper.rb#L262-L265
[3]: https://github.com/rails/sprockets-rails/blob/118ce60b1ffeb7a85640661b014cd2ee3c4e3e56/lib/sprockets/rails/helper.rb#L84
  • Loading branch information
rmacklin committed Nov 25, 2021
1 parent 0e40dca commit 8c4c5aa
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 13 deletions.
2 changes: 1 addition & 1 deletion lib/propshaft/helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Propshaft::Helper
def compute_asset_path(path, options = {})
Rails.application.assets.resolver.resolve(path)
Rails.application.assets.resolver.resolve(path) || raise(Propshaft::MissingAssetError.new(path))
end
end
2 changes: 0 additions & 2 deletions lib/propshaft/resolver/dynamic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ def initialize(load_path:, prefix:)
def resolve(logical_path)
if asset = load_path.find(logical_path)
File.join prefix, asset.digested_path
else
raise Propshaft::MissingAssetError.new(logical_path)
end
end
end
Expand Down
2 changes: 0 additions & 2 deletions lib/propshaft/resolver/static.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ def initialize(manifest_path:, prefix:)
def resolve(logical_path)
if asset_path = parsed_manifest[logical_path]
File.join prefix, asset_path
else
raise Propshaft::MissingAssetError.new(logical_path)
end
end

Expand Down
6 changes: 2 additions & 4 deletions test/propshaft/resolver/dynamic_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ class Propshaft::Resolver::DynamicTest < ActiveSupport::TestCase
@resolver.resolve("one.txt")
end

test "resolving missing asset raises a custom error" do
assert_raise Propshaft::MissingAssetError do
assert_nil @resolver.resolve("nowhere.txt")
end
test "resolving missing asset returns nil" do
assert_nil @resolver.resolve("nowhere.txt")
end
end
6 changes: 2 additions & 4 deletions test/propshaft/resolver/static_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ class Propshaft::Resolver::StaticTest < ActiveSupport::TestCase
@resolver.resolve("one.txt")
end

test "resolving missing asset raises a custom error" do
assert_raise Propshaft::MissingAssetError do
assert_nil @resolver.resolve("nowhere.txt")
end
test "resolving missing asset returns nil" do
assert_nil @resolver.resolve("nowhere.txt")
end
end

0 comments on commit 8c4c5aa

Please sign in to comment.