-
Notifications
You must be signed in to change notification settings - Fork 176
fix: add Vary header to cache interceptor #916
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
Conversation
🦋 Changeset detectedLatest commit: 64ee772 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Co-authored-by: khuezy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I thought I saw something about nextjs putting the vary header back in the newer release? Do we need to patch it on a specific version?
Yeah they did, but they added it back in version Edit: This is also a really hard edge case to hit only in Firefox, and it does require you to have a redirect in the middleware to a route with RSC. Firefox caches the |
The reason why I did not formally approve is that I have no clue if the value for the vary header will fix all cases. |
We could expand the playwright tests to include the other browsers too. |
Closes opennextjs/opennextjs-cloudflare#750
There is some issue with Firefox and possibly some CDNs that they will incorrectly be reusing RSC responses during bfcache restores. Adding a
Vary
header seems to solve the problem.