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
304 response should contain etag #3313
Conversation
|
✔️ Deploy Preview for kit-demo canceled. 🔨 Explore the source changes: 905d5b7 🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61df8ec687fc47000710f502 |
|
||
if (if_none_match_value === etag) { | ||
return { | ||
status: 304, | ||
headers: {} | ||
headers: response.headers |
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.
I don't think we need to include content-type
or any of the other headers in response.headers
. We can probably move response.headers['etag'] = etag;
back where it was, and do headers: { etag }
here instead.
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.
okay
only etag is added to the response if it is a 304 response |
On a closer reading of the original thread, we do need to preserve headers, but only certain ones. I've updated this branch. Thanks! |
Yes that looks like it would fit the spec, thanks! |
Solves #3286
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpx changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0