From daac587637841501587e3acf95c8279547bf6c94 Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 20 May 2023 10:06:02 +0800 Subject: [PATCH 1/5] Support Vary header, Fix caching for colocated endpoints and pages --- .../kit/src/runtime/server/page/serialize_data.js | 13 ++++++------- .../runtime/server/page/serialize_data.spec.js | 4 ++-- packages/kit/src/runtime/server/respond.js | 15 +++++++++++++++ packages/kit/test/apps/basics/test/client.test.js | 4 +++- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/kit/src/runtime/server/page/serialize_data.js b/packages/kit/src/runtime/server/page/serialize_data.js index f176fdfb41a0..10d01c8a8950 100644 --- a/packages/kit/src/runtime/server/page/serialize_data.js +++ b/packages/kit/src/runtime/server/page/serialize_data.js @@ -46,7 +46,7 @@ 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)) { @@ -54,8 +54,8 @@ export function serialize_data(fetched, filter, prerendering = false) { } 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') varyAny ||= value.trim() === '*'; } const payload = { @@ -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'); diff --git a/packages/kit/src/runtime/server/page/serialize_data.spec.js b/packages/kit/src/runtime/server/page/serialize_data.spec.js index 550cf7e0814c..b70170b5174f 100644 --- a/packages/kit/src/runtime/server/page/serialize_data.spec.js +++ b/packages/kit/src/runtime/server/page/serialize_data.spec.js @@ -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 = ''; @@ -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 diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index a82f48ce1111..a27a102a178d 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -401,6 +401,21 @@ 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'); + if ( + !vary + ?.split(',') + ?.map((v) => v.trim()) + ?.includes('Accept') || + vary === '*' + ) { + response.headers.append('Vary', 'Accept'); + } + } + return response; } diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index b95076c4520e..4916c5b1cfcc 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -639,7 +639,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'); From 78961d943c838431572a45346e81aaf39bc6139f Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 20 May 2023 11:10:35 +0800 Subject: [PATCH 2/5] Add changeset, docs --- .changeset/shy-ears-report.md | 7 +++++++ documentation/docs/20-core-concepts/10-routing.md | 3 ++- packages/kit/src/runtime/server/respond.js | 13 +++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 .changeset/shy-ears-report.md diff --git a/.changeset/shy-ears-report.md b/.changeset/shy-ears-report.md new file mode 100644 index 000000000000..fa15dceb2355 --- /dev/null +++ b/.changeset/shy-ears-report.md @@ -0,0 +1,7 @@ +--- +'@sveltejs/kit': minor +--- + +feat: Support caching of responses with `Vary` header (except for `Vary: *`) + +fix: Include `Vary: Accept` header to fix browser caching of adjacent pages and endpoints diff --git a/documentation/docs/20-core-concepts/10-routing.md b/documentation/docs/20-core-concepts/10-routing.md index 4f10d26c36f0..5cb48bd52e4a 100644 --- a/documentation/docs/20-core-concepts/10-routing.md +++ b/documentation/docs/20-core-concepts/10-routing.md @@ -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 diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index a27a102a178d..2b2d100aaf4c 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -404,14 +404,11 @@ export async function respond(request, options, manifest, state) { // 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'); - if ( - !vary - ?.split(',') - ?.map((v) => v.trim()) - ?.includes('Accept') || - vary === '*' - ) { + const vary = response.headers + .get('vary') + ?.split(',') + ?.map((v) => v.trim().toLowerCase()); + if (!(vary?.includes('accept') || vary?.includes('*'))) { response.headers.append('Vary', 'Accept'); } } From 8199313f8879c1e05e585030ba49caee35b2d568 Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 20 May 2023 12:57:43 +0800 Subject: [PATCH 3/5] Update .changeset/shy-ears-report.md Co-authored-by: S. Elliott Johnson --- .changeset/shy-ears-report.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/shy-ears-report.md b/.changeset/shy-ears-report.md index fa15dceb2355..65650a69cca1 100644 --- a/.changeset/shy-ears-report.md +++ b/.changeset/shy-ears-report.md @@ -2,6 +2,6 @@ '@sveltejs/kit': minor --- -feat: Support caching of responses with `Vary` header (except for `Vary: *`) +feat: support caching of responses with `Vary` header (except for `Vary: *`) -fix: Include `Vary: Accept` header to fix browser caching of adjacent pages and endpoints +fix: include `Vary: Accept` header to fix browser caching of adjacent pages and endpoints From cf311037e928ce6532f369188292531f9148e1bf Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 20 May 2023 13:17:33 +0800 Subject: [PATCH 4/5] PR Feedback - Update serialize_data.js --- packages/kit/src/runtime/server/page/serialize_data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/page/serialize_data.js b/packages/kit/src/runtime/server/page/serialize_data.js index 10d01c8a8950..b898a8a4e910 100644 --- a/packages/kit/src/runtime/server/page/serialize_data.js +++ b/packages/kit/src/runtime/server/page/serialize_data.js @@ -55,7 +55,7 @@ export function serialize_data(fetched, filter, prerendering = false) { if (key === 'cache-control') cache_control = value; else if (key === 'age') age = value; - else if (key === 'vary') varyAny ||= value.trim() === '*'; + else if (key === 'vary' && value.trim() === '*') varyAny = true; } const payload = { From a3276e8bb3bce4435e1feb9c9ef890f0f9b3ccd2 Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 27 May 2023 12:42:09 +0800 Subject: [PATCH 5/5] PR feedback: split changeset into two files --- .changeset/shy-ears-report.md | 2 -- .changeset/strange-ladybugs-judge.md | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 .changeset/strange-ladybugs-judge.md diff --git a/.changeset/shy-ears-report.md b/.changeset/shy-ears-report.md index 65650a69cca1..8f955038d8e8 100644 --- a/.changeset/shy-ears-report.md +++ b/.changeset/shy-ears-report.md @@ -3,5 +3,3 @@ --- feat: support caching of responses with `Vary` header (except for `Vary: *`) - -fix: include `Vary: Accept` header to fix browser caching of adjacent pages and endpoints diff --git a/.changeset/strange-ladybugs-judge.md b/.changeset/strange-ladybugs-judge.md new file mode 100644 index 000000000000..9d8f383483a2 --- /dev/null +++ b/.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