propshaft icon indicating copy to clipboard operation
propshaft copied to clipboard

Asset digest is computed before compilation

Open grncdr opened this issue 2 years ago • 6 comments

This is the underlying cause of #90 but the description and comments there conflate a number of issues.

Steps to reproduce

Create two CSS files:

/* application.css */
import('other.css');
/* other.css */
body {
  background-color: pink;
}

Load application.css:

<%= stylesheet_link_tag 'application' %>

Load the page (with browser caching enabled) and observe a pink background.

Change the body background in other.css:

-  background-color: pink;
+  background-color: red;

Reload the page.

Expected result

The background is now red.

Actual result

The background is still pink.

Why this happens

The digest for application.css is computed from the unprocessed file content. This causes Propshaft::Asset to produce the same digest for the asset even though the actual contents at that URL will be different (due to the changing digest of other.css).

Because the asset is served with a very long expiry, browsers will hang on to the outdated version of the asset for a very long time.

Why that's a problem

In a production scenario, browsers would never get the new version of other.css. This makes using @import a pretty big foot gun.

grncdr avatar Dec 20 '22 13:12 grncdr

Just wanted to come back to this issue and confirm that it definitely affects production deployments, because the browser will use a cached version of the stylesheet with older content hashes for the imported files.

We are currently working around the issue by adding a cache-busting comment to the "root" stylesheet before running assets:precompile in CI, but a more robust solution from propshaft would be very appreciated.

grncdr avatar Feb 23 '23 15:02 grncdr

@brenogazzola We'd need to introduce two digests to deal with this, yeah? Basically a raw source digest and a processed source digest. And then keep those straight for when to use.

dhh avatar Mar 04 '23 17:03 dhh

@dhh I think it's a bit more complicated than that? What about multiple levels of import?

/* application.css */
import('elements.css');
/* elements.css */
import('avatar');
import('button');
import('form');
/* buttons.css */
.button {
  background-color: red;
}

If I understand your comment, in this applementation of raw/processed digests, a change in red would cause the raw of buttons to change, but not of elements and application. Then the processed of buttons would change (due to the change in its file) and the processed of elements would change (due to the adjusted import url), but the processed of application would not (since none of its imports had changes in the raw digest)?

I can think of two options: 1 - Keep digesting/redigesting until no digests change 2 - Teach propshaft digest/compile things in order

brenogazzola avatar Mar 06 '23 22:03 brenogazzola

but the processed of application would not (since none of its imports had changes in the raw digest)?

The processed digest of application should depend on the processed digests of its imports, not the raw digests. I guess this is essentially your second option "teach propshaft to compile things in order". (As a bonus, propshaft needs to error on circular dependencies in order for this to be reliable).

grncdr avatar Mar 07 '23 07:03 grncdr

Just to chime in here with additional context/colour on this issue, we migrated to using propshaft on our app for a period but made the decision to revert back to sprockets because of this lack of dependency-resolution in the digest calculation: it caught us out more than once that changes in an imported CSS file was not resulting in a change in the digest.

tekin avatar Aug 27 '23 09:08 tekin

I wasn’t aware sprockets could handle that. I’ll take a look at its source code to see it I can do something similar here. Thanks

brenogazzola avatar Aug 27 '23 17:08 brenogazzola

Fixed by https://github.com/rails/propshaft/pull/188

dhh avatar May 18 '24 00:05 dhh