Skip to content

feat: Support caching of responses with Vary header, and fix browser caching of adjacent pages/endpoints #9993

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

Merged
merged 6 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/shy-ears-report.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 "attr" & a �';
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
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,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
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,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