Skip to content

Commit f4ff87d

Browse files
committed
[Fix #95] Raise When Dependencies Improperly Used
If a dependency is used in an ERB asset that references another asset, it will not be updated when the reference asset is updated. The fix is to use `//= depend_on` or its cousin `//= depend_on_asset` however this is easy to forget. See #95 for more information. Currently Rails/Sprockets hides this problem, and only surfaces it when the app is deployed with precompilation to production multiple times. We know that you will have this problem if you are referencing assets from within other assets and not declaring them as dependencies. This PR checks if you've declared a given file as a dependency before including it via `asset_path`. If not a helpful error is raised: ``` Asset depends on 'bootstrap.js' to generate properly but has not declared the dependency Please add: `//= depend_on_asset "bootstrap.js"` to '/Users/schneems/Documents/projects/codetriage/app/assets/javascripts/application.js.erb' ``` Implementation is quite simple and limited to `helper.rb`, additional code is all around tests. ATP
1 parent 6a03c31 commit f4ff87d

File tree

6 files changed

+32
-5
lines changed

6 files changed

+32
-5
lines changed

lib/sprockets/rails/helper.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@
55
module Sprockets
66
module Rails
77
module Helper
8+
class DependencyError <StandardError
9+
def initialize(path, dep)
10+
msg = "Asset depends on '#{dep}' to generate properly but has not declared the dependency\n"
11+
msg << "Please add: `//= depend_on_asset \"#{dep}\"` to '#{path}'"
12+
super msg
13+
end
14+
end
15+
816
if defined? ActionView::Helpers::AssetUrlHelper
917
include ActionView::Helpers::AssetUrlHelper
1018
include ActionView::Helpers::AssetTagHelper
@@ -36,7 +44,14 @@ def self.extended(obj)
3644
end
3745
end
3846

47+
def check_dependencies!(dep)
48+
return unless @_dependency_assets
49+
return if @_dependency_assets.detect { |asset| asset.include?(dep) }
50+
raise DependencyError.new(self.pathname, dep)
51+
end
52+
3953
def compute_asset_path(path, options = {})
54+
check_dependencies!(path)
4055
if digest_path = asset_digest_path(path)
4156
path = digest_path if digest_assets
4257
path += "?body=1" if options[:debug]

test/fixtures/error/dependency.js.erb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<%= asset_path("bar.js") %>

test/fixtures/logo.png

82.9 KB
Loading

test/fixtures/url.css.erb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
//= depend_on_asset "logo.png"
12
p { background: url(<%= image_path "logo.png" %>); }

test/fixtures/url.js.erb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
//= depend_on_asset "foo.js"
12
var url = '<%= javascript_path :foo %>';

test/test_helper.rb

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def setup
2121
@view = ActionView::Base.new
2222
@view.extend ::Sprockets::Rails::Helper
2323
@view.assets_environment = @assets
24-
@view.assets_prefix = "/assets"
24+
@view.assets_prefix = "/assets"
2525

2626
# Rails 2.x
2727
unless @view.respond_to?(:config)
@@ -30,10 +30,11 @@ def setup
3030
end
3131

3232
@assets.context_class.assets_prefix = @view.assets_prefix
33-
@assets.context_class.config = @view.config
33+
@assets.context_class.config = @view.config
3434

3535
@foo_js_digest = @assets['foo.js'].digest
3636
@foo_css_digest = @assets['foo.css'].digest
37+
@logo_digest = @assets["logo.png"].digest
3738
end
3839

3940
def test_truth
@@ -169,7 +170,7 @@ def test_stylesheet_path
169170

170171
def test_asset_url
171172
assert_equal "var url = '//assets.example.com/assets/foo.js';\n", @assets["url.js"].to_s
172-
assert_equal "p { background: url(//assets.example.com/images/logo.png); }\n", @assets["url.css"].to_s
173+
assert_equal "p { background: url(//assets.example.com/assets/logo.png); }\n", @assets["url.css"].to_s
173174
end
174175
end
175176

@@ -222,7 +223,7 @@ def test_asset_digest
222223

223224
def test_asset_url
224225
assert_equal "var url = '/assets/foo.js';\n", @assets["url.js"].to_s
225-
assert_equal "p { background: url(/images/logo.png); }\n", @assets["url.css"].to_s
226+
assert_equal "p { background: url(/assets/logo.png); }\n", @assets["url.css"].to_s
226227
end
227228
end
228229

@@ -279,7 +280,7 @@ def test_asset_digest_path
279280

280281
def test_asset_url
281282
assert_equal "var url = '/assets/foo-#{@foo_js_digest}.js';\n", @assets["url.js"].to_s
282-
assert_equal "p { background: url(/images/logo.png); }\n", @assets["url.css"].to_s
283+
assert_equal "p { background: url(/assets/logo-#{@logo_digest}.png); }\n", @assets["url.css"].to_s
283284
end
284285
end
285286

@@ -381,3 +382,11 @@ def test_asset_digest
381382
assert_equal @foo_css_digest, @view.asset_digest("foo.css")
382383
end
383384
end
385+
386+
class ErrorsInHelpersTest < HelperTest
387+
def test_dependency_error
388+
assert_raise Sprockets::Rails::Helper::DependencyError do
389+
@assets['error/dependency.js'].to_s
390+
end
391+
end
392+
end

0 commit comments

Comments
 (0)