[Fix #95] Raise When Dependencies Improperly Used #96
Conversation
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 rails#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
|
Seems 👍 for me. |
|
/cc @josh |
|
👍 |
| @@ -36,7 +44,14 @@ def self.extended(obj) | |||
| end | |||
| end | |||
|
|
|||
| def check_dependencies!(dep) | |||
There was a problem hiding this comment.
def check_dependencies!(dep)
if @_dependency_assets && !@_dependency_assets.detect { |asset| asset.include?(dep) }
raise DependencyError.new(self.pathname, dep)
end
endTo avoid the double return,
This PR exposes `matches_filter` via a public method called `filtered?` for the purposes of detecting if an asset exists in a filter. This is to be used in rails/sprockets-rails#96. This change to sprockets was requested by @rafaelfranca to prevent the interface from accidentally changing or breaking.
This PR exposes `matches_filter` via a public method called `filtered?` for the purposes of detecting if an asset exists in a filter. This is to be used in rails/sprockets-rails#96. This change to sprockets was requested by @rafaelfranca to prevent the interface from accidentally changing or breaking.
[Fix #95] Raise When Dependencies Improperly Used
|
👍 (If only Github had a less verbose comment). Congrats! |
|
👍 🐸 |
|
This deserves a CHANGELOG. A lot of apps are going to start throwing exceptions now. |
Add depend_on_asset as it is required as of rails/sprockets-rails#96
|
Has this been released yet? It looks like there hasn't been a release since |
|
Not yet. I'll cook a new release until Monday |
|
(bump) |
|
2.1.0 was released |
|
@schneems should we still use sprockets_better_errors in our rails 4.0.x apps (< 4.1) or can we just use this new 2.1.0 release of sprockets-rails? Thanks! PS: I can give it a try later this week if that helps. |
|
`sprockets_better_errors should be redundant. If you want to give me a PR On Sun, Apr 6, 2014 at 11:20 PM, Jared Beck notifications@github.comwrote:
|
Add depend_on_asset as it is required as of rails/sprockets-rails#96
Add depend_on_asset as it is required as of rails/sprockets-rails#96
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_onor its cousin//= depend_on_assethowever 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:Implementation is quite simple and limited to
helper.rb, additional code is all around tests.ATP