Instead of complaining, add the dependency automatically #126
Conversation
We need to use *both* `depend_on` and `depend_on_asset`, because the latter (bugfully?) considers changes to the referenced asset's dependencies, but not the file itself (making it entirely useless for an image). In the corresponding test, we call a protected Sprockets method... by design, this isn't something we're supposed to care about / access. But actually manipulating assets to ensure the dependency has taken hold seems rather like overkill.
Instead of complaining, add the dependency automatically
Instead of complaining, add the dependency automatically
|
I don't think this will work in production. When While this sets the dependency flag, it does so only in memory, and this does not persist across multiple runs (and would not show during If I were to re-write sprockets, I would consider storing dependencies in the manifest that way you could programmatically add them and not need to scan the files before precompiling them. |
|
@schneems I really didn't get what is the case where it doesn't work in production. I tested several times in an example application and everything worked fine. Could you explain which cases are these? |
|
@schneems AFAICS, dependencies are part of the info stored in the cache. And if the cache is absent, it's properly rebuilt. (Working in production env,) I can modify an image, and then see the referencing CSS get rebuilt -- even if I manually kill |
|
So i spent a long time proving myself wrong. Thanks for the PR. You are amazing ❤️ ❤️ Here's my script: cd /tmp
rails new foo
cd foo
echo "gem 'sprockets-rails', github: 'rails/sprockets-rails'" >> Gemfile
bundle update sprockets-rails
rm app/assets/javascripts/application.js
echo "var a = '<%= asset_path(\"application.css\") %>';" > app/assets/javascripts/application.js.erb
RAILS_ENV=production rake assets:precompile
echo "body { background-color: red}" >> app/assets/stylesheets/application.css
RAILS_ENV=production rake assets:precompileBoth assets are compiled both times: $ RAILS_ENV=production rake assets:precompile
I, [2014-04-09T11:10:22.277067 #3009] INFO -- : Writing /private/tmp/foo/public/assets/application-8b4807285af38137c51b3b2edaf73099.js
I, [2014-04-09T11:10:22.278046 #3009] INFO -- : Writing /private/tmp/foo/public/assets/application-d0b54dd563966c42aad5fd85b1c1f713.css
2.1.1 /tmp/foo
$ echo "body { background-color: red}" > app/assets/stylesheets/application.css
2.1.1 /tmp/foo
$ RAILS_ENV=production rake assets:precompile
I, [2014-04-09T11:10:25.150878 #3039] INFO -- : Writing /private/tmp/foo/public/assets/application-2ac6634a2b603e075d1b46bcc69f03e3.js
I, [2014-04-09T11:10:25.151782 #3039] INFO -- : Writing /private/tmp/foo/public/assets/application-deb47372583b3c66395a0cf56fa0011b.cssGreat work! I wish I knew better how sprockets was doing this under the covers. Heroku has an LRU script that cleans up old files, and I believe something similar just got committed to sprockets master, I hope we don't accidentally just delete the dependency information and screw up this caching behavior. In addition to this PR, we'll need to remove the asset flag and update all the docs to show that it isn't needed. |
Use an explicit constant, so we don't invalidate caches for every minor bump of the gem... but we can do so when the need arises. Adding this value obviously counts as a bump & invalidation: that's desirable, so rails#126 can start from a clean slate.
|
Documentation updated at rails/rails@b053a47. The flag still make sense since we are using to detect missing assets on precompile configuration. |
After trying this gem and following the github readme, I noticed that the gem isn't referencing "Jcrop.gif". Unless I'm missing something, this file (and so the minify mersion) needs this line.
|
I wonder if I mis-interpreted this fix. Would we still need to declare our dependencies using depend_on_asset? I'm still getting errors using version 2.1.3: |
|
Make sure you're not also using If we can reproduce some of those unusual instances, i would be interested in knowing about their root causes. |
|
It was in fact sprockets_better_errors Thanks! On Saturday, June 21, 2014, Richard Schneeman notifications@github.com
|
It seems we can easily make #125 Just Work.
Experimentally, I've determined that:
depend_on_assetin a.css.scssfile to match animage-urlis sufficient to make the error go away, but is not sufficient to cause the generated CSS to be rebuilt when the image content changesdepend_on_assetis required, and it Just Works.Am I just being even more obtuse than usual? Is there some particular arrangement where this doesn't work, and we must insist that the author give us the information?
We need to use both
depend_onanddepend_on_asset, because the latter (bugfully?) considers changes to the referenced asset's dependencies, but not the file itself (making it entirely useless for an image).I'll follow up on that point with Sprockets, but given that we're currently locked very firmly on a particular version, and that it'll remain a safe no-op if
depend_on_assetstarts to do the job ofdepend_ontoo, it seems best for us to just call both -- at least for now.Suggestions to avoid calling a Sprockets-protected method are welcome... though again, right now, we know we're breaching Sprockets intended public API. And I suspect that would apply to the current usage of
_dependency_assets, too.@rafaelfranca @schneems