jwhitley / requirejs-rails

RequireJS support for your Rails 3 or 4 application
MIT License
592 stars 202 forks source link

Generate the asset hash for a file based on its built contents, not t… #238

Closed jonhyman closed 9 years ago

jonhyman commented 9 years ago

…he static contents from sprockets.


In https://github.com/jwhitley/requirejs-rails/pull/217, @koenpunt switched to copying the built file to asset.digest_path. However, this is the digest of the original source file. If a dependency of one of the files changes, then the hash will remain the same. This causes massive problems with browsers not getting the new contents because the filename is the same and the browser pulls from cache.

koenpunt commented 9 years ago

Yeah that was one of my concerns at that time.. (https://github.com/jwhitley/requirejs-rails/pull/217#issuecomment-94054054) Great you fixed it.

carsomyr commented 9 years ago

@jonhyman Thanks for this! Let me do a git log -p and see what, exactly, was removed in favor of asset.digest_path.

lsimoneau commented 9 years ago

Just got bit by this issue, serving up a cached version when we'd made modifications to one of the dependencies. Is there anything I can do to help get this merged?

carsomyr commented 9 years ago

@lsimoneau Sorry for the delay. I'll get onto it within the next two days.

lsimoneau commented 9 years ago

Any updates on this? This keeps biting us.

carsomyr commented 9 years ago

@lsimoneau I'm inundated with work right now. Will try to get to it in the next few days. Why not switch your Gemfile over to using your personal repo temporarily?

jonhyman commented 9 years ago

@lsimoneau we're using my fork/branch in prod and it works well

lsimoneau commented 9 years ago

No worries, I'll switch to a fork. Again, if there's anything I can do to help (adding tests? something else?) let me know.

carsomyr commented 9 years ago

All, I'm releasing a new version. Let me know how it goes.

ketan commented 9 years ago

This change seems to have broken when Sprockets::DigestUtils.pack_hexdigest is not available because of an older version of sprockets (2.3.1) running from rails 4.0.

Unfortunately upgrading sprockets is not an option, because it appears to break other dependencies.

Is there an alternative that I can use? Happy to submit a patch.

koenpunt commented 9 years ago

Sprockets 2 has Sprockets::Base.file_digest() and Sprockets::Asset.digest_path()

jonhyman commented 9 years ago

Can you fork and try doing something like

if defined?(Sprockets::DigestUtils) && Sprockets::DigestUtils.respond_to?(:pack_hexdigest)
  hex_digest = Sprockets::DigestUtils.pack_hexdigest(file_digest)
else
 hex = Sprockets::Base.file_digest(file_digest)
end

and see if that works for you?