-
Notifications
You must be signed in to change notification settings - Fork 313
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
Vary
and requests added after the SW
#1542
Comments
I'm not sure I understand the issue. The |
It's the other way around. |
Oh, yes. Sorry for my confusion. |
Sorry, what does "below" mean here? Last I understood, Chrome sets the User Agent header before the service worker (i.e., the SW sees the UA header). The spec says the User Agent header should be set after the service worker (i.e., the SW doesn't see the UA header). Related bugs are https://crbug.com/595993, https://crbug.com/962620, https://crbug.com/963260 |
Apologies for the confusing langauge. By "below", I meant "added after SWs". |
Vary
and requests added below the SWVary
and requests added after the SW
We discussed this in last week's call. |
I learned recently that
Cache.match()
is taking the Vary headers into account when matching requests and responses. That seems neat.At the same time, for request headers added after the SW (e.g.
Cookie
,User-Agent
) that doesn't seem like something that can work: The server indicates that the request would vary based on the user state or browser version, but that is not respected because theRequest
objects the SW'sfetch
event sees don't contain those headers and would always match, even if those headers changed in the underlying layers.The reason I'm looking into this is that for Client Hints, we decided on adding them above the SW in order to have visibility into their values when matching responses, but that has many downsides, especially when it comes to SW generated requests.
Maybe the right path here is a way to provide those relevant values to
match()
(coming from the relevant JS APIs) in a generic way that'd enable it to take them into account?The text was updated successfully, but these errors were encountered: