Override Propshaft digesting for shorter file names

[aduth]

🛠 Summary of changes

This explores overriding Propshaft's default digesting logic in order to produce shorter file names. Since these files names are referenced in stylesheets (e.g. url(/public/assets/...)) and are not likely to be well-optimized by DEFLATE/LZ77/Huffman-based compression, it's theorized that they may be contributing to larger overall file size.

The downside of this is it increases the likelihood of a hash collision, though it's quite unlikely, both that a hash collision would occur in the first place, and that two versions of a collided file hash would be cached in a user's browser such that conflicts could occur.

To my knowledge, Propshaft does not expose a way to customize this behavior, so the implementation here monkeypatches the original class method. The logic is largely the same, except the digest is shortened from its original result to take only the first 7 characters.

https://github.com/rails/propshaft/blob/18979c176132f432f70f8d7f7ff97ece035cba5d/lib/propshaft/asset.rb#L23-L25

Performance Impact:

This affects the file names for all compiled assets, notably impacting:

• References to url(...) file paths within CSS files
• Total size of HTML markup from path references to images and stylesheets
• Response preload headers

Results on application.css:

Before: 27.6kb brotli'd, minified
After: 26.7kb brotli'd, minified
Diff: -0.9kb (-3.26%)

Markup size:

curl --compressed -so /dev/null http://localhost:3000 -w '%{size_download}'

Before: 4598 bytes
After: 4155 bytes
Diff: -433 bytes (-9.6%)

"Link" Header size:

curl --silent -I http://localhost:3000 | sed -n -e 's/^Link: //p' | wc -c

Before: 1632 bytes
After: 1467 bytes
Diff: -165 bytes (-10.1%)

📜 Testing Plan

NODE_ENV=production RAILS_ENV=production rails assets:precompile

Observe:

1. Files in public/assets have a fingerprint in file path that is only 7 characters in length (compare to longer on main)
2. Within CSS files (e.g. public/assets/application-*.css), references to other files are accurate and use the shorter fingerprint

[zachmargolis]

does the original method we're overriding also set the @digest ivar? if so... this may not actually override it?

update: looks like yes it does

[zachmargolis]

Suggested change

	@digest ||= original_digest[0...7]
	@shorter_digest ||= original_digest[0...7] # rubocop:disable Naming/MemoizedInstanceVariableName

rubocop will yell because the ivar name doesn't match but we can override

[aduth]

Hm, I thought I had tested this, but checking again, it does look like it reverted back to the original behavior with this change. Maybe it's best to just duplicate as it was in my earlier iteration.

[aduth]

Fixed/reverted in 72ac3b7.

[zachmargolis]

LGTM but if it were up to me, I'd say we skip this?

My thinking:

• it's a tiny bit brittle due to monkeypatching
• Also we have no way to alert/check in the rare case there was ever a SHA collision
• For a the 3% decrease in asset size, maybe not be worth it?

[aduth]

Also we have no way to alert/check in the rare case there was ever a SHA collision

One other aspect of this I hadn't noted previously is that this also brings some consistency to what we're already doing with JavaScript files, which are already digested with a fingerprint limited to just 8 characters.

Let me hold and explore a few things, though I do kinda feel motivated to continue on this:

• Make it truly consistent between JS/CSS (i.e. pick 7 or 8 characters)
• Maybe we can incorporate the fingerprinting into @18f/identity-build-sass ? It doesn't really help from the end-user performance angle of this, but could be more efficient to generate the fingerprint at the time that the .scss file is being compiled / written to disk, rather than as part of asset precompilation
• See if there's more impact/benefit outside just application.css
• Propose an upstream issue or pull request to Propshaft to make this configurable, to address the concern of brittleness

[aduth]

Propose an upstream issue or pull request to Propshaft to make this configurable, to address the concern of brittleness

Created upstream pull request at rails/propshaft#155

[aduth]

See if there's more impact/benefit outside just application.css

Since this affects the file names for all compiled assets, this also has some benefit on the total size of HTML markup (image and stylesheet tags), as well the preload headers we include in responses.

Some quick testing of before/after

Markup size:

curl --compressed -so /dev/null http://localhost:3000 -w '%{size_download}'

Before: 4598 bytes
After: 4155 bytes
Diff: -433 bytes (-9.6%)

"Link" Header size:

curl --silent -I http://localhost:3000 | sed -n -e 's/^Link: //p' | wc -c

Before: 1632 bytes
After: 1467 bytes
Diff: -165 bytes (-10.1%)

[aduth]

Make it truly consistent between JS/CSS (i.e. pick 7 or 8 characters)

Updated to 8 characters in 47bab96 to be consistent with JavaScript digest (and coincidentally also component ID generation).

I tested this in a review-app to get a sense of how this should behave in a production-like environment and it looks good there.

Unless there's opposition, I'll plan to merge this in the morning.