Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Commit

Permalink
Generate the asset hash for a file based on its built contents, not t…
Browse files Browse the repository at this point in the history 
…he static contents from sprockets.
  • Loading branch information
@jonhyman
jonhyman committed Jun 24, 2015
1 parent 2f79626 commit c7babb6
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/tasks/requirejs-rails_tasks.rake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ OS X Homebrew users can use 'brew install node'.
asset = requirejs.env.find_asset(asset_name)

built_asset_path = requirejs.config.build_dir.join(asset_name)
digest_name = asset.digest_path

file_digest = ::Rails.application.assets.file_digest(built_asset_path)
hex_digest = Sprockets::DigestUtils.pack_hexdigest(file_digest)
digest_name = asset.logical_path.sub(/\.(\w+)$/) { |ext| "-#{hex_digest}#{ext}" }

digest_asset_path = requirejs.config.target_dir + digest_name

# Ensure that the parent directory `a/b` for modules with names like `a/b/c` exist.
Expand Down

3 comments on commit c7babb6

@agis
Copy link
Contributor

@agis agis commented on c7babb6 Aug 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit implicitly adds a dependency on Sprockets 3, which added the DigestUtils class. I think we'll be better off by duplicating the method, ie:

- hex_digest = Sprockets::DigestUtils.pack_hexdigest(file_digest)
+ hex_digest = file_digest.to_s.unpack('H*').first

WDYT? I could open a PR if it looks good.

@bteixeira
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

This is breaking asset pre-compilation with Sprockets 2 which will force me to either use requirejs-rails 0.9.5 or do a careful migration and possibly painful to Sprockets 3.

@agis- solution seems like the way to go, but otherwise at least add an explicit dependency on the new Sprockets version

@agis
Copy link
Contributor

@agis agis commented on c7babb6 Aug 22, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bteixeira IIRC there are more commits that break compatibility with Sprockets 3; those introduced in #217.

If you're going to use 0.9.5 (which works with Sprockets 2) and use a custom manifest file, you probably want to use this fork: https://github.com/skroutz/requirejs-rails.

Please sign in to comment.