Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make environment version affect asset digest #404

Merged
merged 1 commit into from Oct 17, 2016

Conversation

lime
Copy link
Contributor

@lime lime commented Oct 14, 2016

Based on discussion in rails/sprockets-rails#240, this is an attempt to bring back the behaviour of Sprockets 2.

The intended use case is to force every asset to be regenerated with a new filename by bumping the environment version. This is useful for example when serving assets from a CDN with long expiration times.

The old behaviour was removed in 07e5e29. The entire digest setup has changed a lot since then. It's not entirely clear to me where the best spot now would be to reintroduce the version – should it affect all digest calculation throughout the library, or just the digest which ends up as part of the filename?

I've started by making a very minimal change that would achieve the intended behaviour. I assume that this will not be the version that gets merged, but maybe it can serve as a starting point. :) Let me know your thoughts.

@matthewd
Copy link
Member

At a glance, this sounds pretty reasonable to me. I believe we want to do two things:

  1. Change the filenames on the final compiled assets
  2. Run the original asset source through all the processors again

And this sounds like it will do both.

(We should add a test of the latter, though)

@schneems
Copy link
Member

I'm fine with this, kind of baffled why it was removed. Why have a user set-able version if it does nothing?

@schneems
Copy link
Member

Thanks for the PR, going to go ahead and merge. Can you send me another PR with a test for ensuring that the asset gets re-run through other processors?

@schneems schneems merged commit 0488a02 into rails:master Oct 17, 2016
@lime
Copy link
Contributor Author

lime commented Oct 18, 2016

I'd be happy to try, though I'm not really sure how to test it. Maybe you can give me some pointers in the right direction?

It may even be that the run-through-all-processors behaviour has been intact all along. That's the way I interpreted the discussion in rails/sprockets-rails#240 (comment). Definitely agree that it should be tested if it isn't already.

@matthewd
Copy link
Member

My first thought is an erb-processed asset of the form:

var version = <%= something?.env.version %>;

That would just prove that as well as changing the filename, we've successfully rebuilt the file from scratch -- no intermediate cache keeping anything old alive.

@jacobbednarz
Copy link
Contributor

@schneems Just checking, is the master branch here for the 4.x release? Or the 3.x? I see this has landed in master but interested in using on 3.x and it's not 100% clear which "next version" this one will land in.

@jacobbednarz
Copy link
Contributor

straight after updating this issue, I seen it is tagged for the v4.0.0.beta4 release 🙄 Is this something you'd be interested in someone backporting to 3.x?

jacobbednarz added a commit to jacobbednarz/sprockets that referenced this pull request May 2, 2017
Updates the file digest to take into account the configured version
which allows tasks like invalidating all assets possible again.

Backports rails#404 to 3.x
@jacobbednarz
Copy link
Contributor

Seeing how we needed this functionality for 3.x, I've gone ahead and opened #478 in case you want to merge this.

jacobbednarz added a commit to jacobbednarz/sprockets that referenced this pull request May 16, 2017
Updates the file digest to take into account the configured version
which allows tasks like invalidating all assets possible again.

Backports rails#404 to 3.x using patch from sellect#1
jacobbednarz added a commit to jacobbednarz/sprockets that referenced this pull request May 16, 2017
Updates the file digest to take into account the configured version
which allows tasks like invalidating all assets possible again.

Backports rails#404 to 3.x using patch from sellect#1
@rafbm
Copy link
Contributor

rafbm commented Jun 3, 2020

This change only covered assets with a preprocessor, which notably excludes images. I attempted a fix in #680.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants