ability to specify cache-control header in config

[equivalent]

Ability to tell what cache-control header we want the assets loaded with

This will solve the issue #90

Solution proposed by @dhh in issue 94 :

two things here:

1. no-cache header

no-cache works but maybe no-store is more what we want (source)

... I don't mind either

2. this could be simpler?

I'm not sure if I didn't overkill this. Maybe something simpler would be enough eg:

# lib/propshaft/server.rb
       {
          #...
          "cache-control"   => Rails.development? ? 'no-store' : "public, max-age=31536000, immutable"
       }

...let me know I can simplify it

[equivalent]

🤔 wait a minute, now I'm re-reading @brenogazzola comment

...hmm I'm not sure about the prod but this solved the dev isuue 🤔

screen recording 📹 👇
https://user-images.githubusercontent.com/721990/210008614-65a9d72f-f621-4145-b065-22848c9bee3d.mov

[brenogazzola]

Video looks a bit strange to me. The digest of application.css did not change, which means the @include "my-custom.css" should still be pointing to 8ffbe1. So how did the browser know it had to fetch f4b376 after refresh?

[equivalent]

@brenogazzola

Ok took me a while to realise what's happening:

• yes application-xxxxxxxx.css digest don't change
• yes I agree theoretically it should (as digest is calculated from content and @import url('/my-custom.css'); is translated to @import url('/my-custom-mmmmmmm.css'); = application.css content changed so it should be application-yyyyyy.css ... but it's not!)

To see what I mean please watch https://youtu.be/cqQHyJj7few

So because browser don't cache application-xxxxxxxx.css it will load it from server every time and the next time loaded it poins to different @import url('/my-custom-nnnnnnnn.css'); => no issue like #90

how come the digest of application-xxxxxxxx.css didn't change ? 🤷‍♂️ my guess is that the digest of application.css is calculated on a file content before content gets digested (before my-custom.css turns to my-custom-nnnnnn.css

[equivalent]

one more thing - during testing the above I've stumbled upon an edegecase:

My PR proposes the no-store header by default for Development env(therefor no such issue as #90) 👍.

If a developer switch the non-cache header to any form of cache header and then switch back to no-store again, his/her browser will be stuck at old cache.

To see this in action pls watch https://youtu.be/3nmCubctHuk

Given on cache header browser will cache the old application-xxxxx.css
When I switch to no-store header browser still cache the old application-xxxxx.css
Therefore it will not pick up the fact the my-custom.css has a new digest

conclusion:

• My PR works 👍
• My PR solves change is not detected in @import css file  #90 issue in development (tat means first time users will not have that issue as Development env fixed by default) 👍
• If someone tries to test if cache works, they need to clear the browser cache or do a change in application.css (so new digest appears for application-yyyy.css) 😕

[simi]

💪 I can confirm this fixes the problem. And yes, I had to wipe the cache locally first to get this change effective.

[simi]

Does it make sense to have this value out-of-sync with config.assets.sweep_cache?
Wouldn't be easier to keep it in sync by default like

config.assets.cache_control_header = config.assets.sweep_cache ? "no-store" : "public, max-age=31536000, immutable"

❓

[equivalent]

yep sweep_cache makes more sense 👍, I will change it later this week 😇

[equivalent]

@simi on second thought this may be a bad idea

by relying on value of config.assets.sweep_cache we may end up overiding the value config.assets.cache_control_header if user wants to set it himself/herself:

Example:

# config/evvironments/development.rb
#...
config.assets.cache_control_header = "whatever"
config.assets.sweep_cache = true 
#...

...now what ?? Should the cache_control_header be no-store or whatever ?

I would say the original PR is least prone to edgecases = let the developer decide what value to return as cache_control_header

Please watch full explanation on loom:

[simi]

Clear, this could be fixed by setting original value of sweep_cache to nil by default and only when it is not assigned to false explicitly, change cache_control_header and also self assign to false. Anything in here works for me. Even two independent config options as originally proposed. With sane default values per environment it would be rarely tweaked by users anyway.

[equivalent]

@simi Sorry I'm getting back after 2 weeks (I had some personal issues to solve)

Thank you for the recommendation how to fix this with nil sweep_cache. I understand

Even two independent config options as originally proposed

Unless you really want the sweep_cache nil solution (as you proposed) I would vote for the original "two independent config options" solution. I get it no-one will really change the values. But personally purely to have simple straight forward config.

please let me know if I should do the nil change proposed or not

[simi]

As mentioned no huge preference from my side. Let's grab some maintainers attention somehow, they can decide on their own. 🙏

@brenogazzola @dhh friendly ping

[brenogazzola]

how come the digest of application-xxxxxxxx.css didn't change ? 🤷‍♂️ my guess is that the digest of application.css is calculated on a file content before content gets digested (before my-custom.css turns to my-custom-nnnnnn.css

I'm a bit worried about this part. Yes, you are right, parent digest is calculated on the original content, not the one that was changed with the digest of the children imports.

This means this solution will give you a false impression that everything is working because in the development you are seeing the changes. But when you push to production, anyone who had already fetched the parent file (eg: your CDN which is sending the files to your users) will not fetch again it again, since its digest did not change and they think it's the same file.

This means that if you are using new classes that were added to the children files, your site is now broken (at least visually).

The only feasible solution I see is one that ensures that parent digest changes when children digest change

[dhh]

We need to fix this at the root. The no-cache setup isn't going to work for production either. Need to invalidate files when their dependencies have changed. There are several open approaches for that, so exploring those.