-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make it more clear that cache mode "default" does respect Cache-Control headers and such #336
Comments
Hmm, wouldn't this be observable through a service worker script which intercepts the original fetch and looks at the |
I thought so too, but where we modify it today is way later. Basically just before asking the HTTP cache. Which seems fairly sensible. |
However, it's probably better to not special case headers and header values since we might introduce new headers and header values and "default" should remain working the same way. The current switch from "default" to something else is largely a legacy thing. |
Can't the service worker just inspect the headers anyway? Why are we worried about it observing it? |
I think we're only worried to the extent that it would affect existing implementations. |
If the cache mode is “default” and you set a Cache-Control header, that should be taken into account when talking to the HTTP cache subsystem. It’s how the cache works by “default”, after all. Fixes #336.
Basically, when you go to the HTTP cache with a request whose cache mode is "default", and the request also has Cache-Control or Pragma headers, those need to be respected by the HTTP cache, just as they always were.
Alternatively we could make those headers and their values change the cache mode:
Cache-Control
orPragma
Header is present, whose value is “no-cache”, then set the cache mode from “Default” to no-cache.Cache-Control
orPragma
Header is present, whose value is “no-store”, then change the cache mode from “Default” to no-store.That's not observable so either approach is probably fine. Thoughts anyone?
The text was updated successfully, but these errors were encountered: