Skip to content

Commit 03533fe

Browse files
committed
Deprecate asset fallback
When an asset isn't found the behavior is to pass the string through. For example a valid asset will return a url from pipeline ``` asset_path("application.js") # => assets/application-123098udasvi0mnafd.js ``` While if you make a typo, you won't get an error or anything, it just falls through: ``` asset_path("app1icati0n.js") # => "app1icati0n.js" ``` Hopefully I don't have to elaborate on why this is bad. This PR is a child PR to one in Rails that will introduce a `public_asset_path` API. There are valid reasons for not using the asset pipeline, if you have a purely static asset in the public folder or if you want to link to a static URL in your assets somewhere it makes sense to declare that intention. Eventually we will replace the behavior of the deprecation with an exception so people don't lose hours of their to typos. We only emit the deprecation when a `public_` api is available in Rails. This means that sprockets-rails can still be used in a backwards compatible manner.
1 parent 7fa2b4d commit 03533fe

File tree

2 files changed

+99
-1
lines changed

2 files changed

+99
-1
lines changed

lib/sprockets/rails/helper.rb

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,35 @@ def assets_environment
6868
end
6969
end
7070

71+
# Writes over the built in ActionView::Helpers::AssetUrlHelper#compute_asset_path
72+
# to use the asset pipeline.
7173
def compute_asset_path(path, options = {})
7274
debug = options[:debug]
7375

7476
if asset_path = resolve_asset_path(path, debug)
7577
File.join(assets_prefix || "/", legacy_debug_path(asset_path, debug))
7678
else
77-
super
79+
result = super
80+
if respond_to?(:public_asset_path)
81+
throw(:asset_not_found, result)
82+
else
83+
result
84+
end
85+
end
86+
end
87+
88+
# Writes over the built in ActionView::Helpers::AssetUrlHelper#asset_path
89+
# to use the asset pipeline.
90+
def asset_path(*args)
91+
catch_asset_not_found = catch(:asset_not_found) do
92+
return super(*args)
7893
end
94+
95+
result = catch_asset_not_found
96+
deprecate_invalid_asset_lookup(result, caller)
97+
result
7998
end
99+
alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route
80100

81101
# Resolve the asset path against the Sprockets manifest or environment.
82102
# Returns nil if it's an asset we don't know about.
@@ -244,6 +264,49 @@ def legacy_debug_path(path, debug)
244264
path
245265
end
246266
end
267+
268+
private
269+
# Attempts to extract a method name from a given caller line
270+
#
271+
# Example:
272+
#
273+
# extractd_method_from_call_frame('console.rb:65:in `start'') => 'start'
274+
def extract_method_from_call_frame(frame)
275+
frame.split("in ".freeze).last.gsub(/`|'/, ''.freeze)
276+
end
277+
278+
# Emits a deprecation warning when asset pipeline
279+
# is used with an asset that is not part of the pipeline.
280+
#
281+
# Attempts to determine proper method name based on caller.
282+
def deprecate_invalid_asset_lookup(name, call_stack)
283+
message = "The asset #{ name.inspect } you are looking for is not present in the asset pipeline.\n"
284+
message << "The public fallback behavior is being deprecated and will be removed.\n"
285+
286+
append = nil
287+
# Search for the most likely top level method where deprecation occured.
288+
# Hash contains, in order, the suffix we're looking for, and where it would appear
289+
# in the call stack.
290+
# This is needed because every deprecated method eventually calls `asset_path`.
291+
{ "_url" => [0, 1] , "_tag" => [1, 2], "_path" => [0] }.detect do |suffix, positions|
292+
293+
positions.detect do |position|
294+
public_method_name = "public_" + extract_method_from_call_frame(call_stack[position])
295+
296+
if public_method_name.end_with?(suffix) && respond_to?(public_method_name)
297+
position.times { call_stack.shift }
298+
append = "please use the `public_*` helper instead. For example `#{ public_method_name }`.\n"
299+
else
300+
false
301+
end
302+
end
303+
end
304+
305+
append ||= "please use the `public_*` helper instead for example `public_asset_path`.\n"
306+
message << append
307+
308+
ActiveSupport::Deprecation.warn(message, call_stack)
309+
end
247310
end
248311

249312
# Use a separate module since Helper is mixed in and we needn't pollute

test/test_helper.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,41 @@ def test_index_files
827827
end
828828
end
829829

830+
class DeprecationTest < HelperTest
831+
def test_deprecations_for_asset_path
832+
@view.send(:define_singleton_method, :public_asset_path, -> {})
833+
assert_deprecated(/public_asset_path/) do
834+
@view.asset_path("does_not_exist.noextension")
835+
end
836+
ensure
837+
@view.instance_eval('undef :public_asset_path')
838+
end
839+
840+
def test_deprecations_for_asset_url
841+
@view.send(:define_singleton_method, :public_asset_path, -> {})
842+
@view.send(:define_singleton_method, :public_asset_url, -> {})
843+
844+
assert_deprecated(/public_asset_url/) do
845+
@view.asset_url("does_not_exist.noextension")
846+
end
847+
ensure
848+
@view.instance_eval('undef :public_asset_path')
849+
@view.instance_eval('undef :public_asset_url')
850+
end
851+
852+
def test_deprecations_for_image_tag
853+
@view.send(:define_singleton_method, :public_asset_path, -> {})
854+
@view.send(:define_singleton_method, :public_image_tag, -> {})
855+
856+
assert_deprecated(/public_image_tag/) do
857+
@view.image_tag("does_not_exist.noextension")
858+
end
859+
ensure
860+
@view.instance_eval('undef :public_asset_path')
861+
@view.instance_eval('undef :public_image_tag')
862+
end
863+
end
864+
830865
class RaiseUnlessPrecompiledAssetDisabledTest < HelperTest
831866
def test_check_precompiled_asset_enabled
832867
@view.check_precompiled_asset = true

0 commit comments

Comments
 (0)