Skip to content

Commit

Permalink
feat: better Vary header support (#9993)
Browse files Browse the repository at this point in the history
- support caching of responses with `Vary` header (possible without any changes on the client because since #8754 we're taking headers into account for the cache key)
- fix browser caching of adjacent pages/endpoints

fixes #9780

---------

Co-authored-by: S. Elliott Johnson <sejohnson@torchcloudconsulting.com>
  • Loading branch information
oscarhermoso and tcc-sejohnson committed Jul 4, 2023
1 parent f2c6e4b commit 42c9b93
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/shy-ears-report.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': minor
---

feat: support caching of responses with `Vary` header (except for `Vary: *`)
5 changes: 5 additions & 0 deletions .changeset/strange-ladybugs-judge.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: include `Vary: Accept` header to fix browser caching of adjacent pages and endpoints
3 changes: 2 additions & 1 deletion documentation/docs/20-core-concepts/10-routing.md
Expand Up @@ -330,7 +330,8 @@ export async function POST({ request }) {
`+server.js` files can be placed in the same directory as `+page` files, allowing the same route to be either a page or an API endpoint. To determine which, SvelteKit applies the following rules:
- `PUT`/`PATCH`/`DELETE`/`OPTIONS` requests are always handled by `+server.js` since they do not apply to pages
- `GET`/`POST` requests are treated as page requests if the `accept` header prioritises `text/html` (in other words, it's a browser page request), else they are handled by `+server.js`
- `GET`/`POST` requests are treated as page requests if the `accept` header prioritises `text/html` (in other words, it's a browser page request), else they are handled by `+server.js`.
- Responses to `GET` requests will inlcude a `Vary: Accept` header, so that proxies and browsers cache HTML and JSON responses separately.
## $types
Expand Down
13 changes: 6 additions & 7 deletions packages/kit/src/runtime/server/page/serialize_data.js
Expand Up @@ -46,16 +46,16 @@ export function serialize_data(fetched, filter, prerendering = false) {

let cache_control = null;
let age = null;
let vary = false;
let varyAny = false;

for (const [key, value] of fetched.response.headers) {
if (filter(key, value)) {
headers[key] = value;
}

if (key === 'cache-control') cache_control = value;
if (key === 'age') age = value;
if (key === 'vary') vary = true;
else if (key === 'age') age = value;
else if (key === 'vary' && value.trim() === '*') varyAny = true;
}

const payload = {
Expand Down Expand Up @@ -89,10 +89,9 @@ export function serialize_data(fetched, filter, prerendering = false) {
}

// Compute the time the response should be cached, taking into account max-age and age.
// Do not cache at all if a vary header is present, as this indicates that the cache is
// likely to get busted. It would also mean we'd have to add more logic to computing the
// selector on the client which results in more code for 99% of people for the 1% who use vary.
if (!prerendering && fetched.method === 'GET' && cache_control && !vary) {
// Do not cache at all if a `Vary: *` header is present, as this indicates that the
// cache is likely to get busted.
if (!prerendering && fetched.method === 'GET' && cache_control && !varyAny) {
const match = /s-maxage=(\d+)/g.exec(cache_control) ?? /max-age=(\d+)/g.exec(cache_control);
if (match) {
const ttl = +match[1] - +(age ?? '0');
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/runtime/server/page/serialize_data.spec.js
Expand Up @@ -81,7 +81,7 @@ test('computes ttl using cache-control and age headers', () => {
);
});

test('doesnt compute ttl when vary header is present', () => {
test('doesnt compute ttl when vary * header is present', () => {
const raw = 'an "attr" & a \ud800';
const escaped = 'an &quot;attr&quot; &amp; a &#55296;';
const response_body = '';
Expand All @@ -93,7 +93,7 @@ test('doesnt compute ttl when vary header is present', () => {
request_body: null,
response_body,
response: new Response(response_body, {
headers: { 'cache-control': 'max-age=10', vary: 'accept-encoding' }
headers: { 'cache-control': 'max-age=10', vary: '*' }
})
},
() => false
Expand Down
12 changes: 12 additions & 0 deletions packages/kit/src/runtime/server/respond.js
Expand Up @@ -402,6 +402,18 @@ export async function respond(request, options, manifest, state) {
throw new Error('This should never happen');
}

// If the route contains a page and an endpoint, we need to add a
// `Vary: Accept` header to the response because of browser caching
if (request.method === 'GET' && route.page && route.endpoint) {
const vary = response.headers
.get('vary')
?.split(',')
?.map((v) => v.trim().toLowerCase());
if (!(vary?.includes('accept') || vary?.includes('*'))) {
response.headers.append('Vary', 'Accept');
}
}

return response;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/kit/test/apps/basics/test/client.test.js
Expand Up @@ -650,7 +650,9 @@ test.describe('data-sveltekit attributes', () => {

test.describe('Content negotiation', () => {
test('+server.js next to +page.svelte works', async ({ page }) => {
await page.goto('/routing/content-negotiation');
const response = await page.goto('/routing/content-negotiation');

expect(response.headers()['vary']).toBe('Accept');
expect(await page.textContent('p')).toBe('Hi');

const pre = page.locator('pre');
Expand Down

0 comments on commit 42c9b93

Please sign in to comment.