Skip to content

Commit

Permalink
[fix] respect fetch cache option (#8024)
Browse files Browse the repository at this point in the history
* [fix] respect fetch cache option

fixes #7971

* fix cache key computation
  • Loading branch information
dummdidumm authored Dec 9, 2022
1 parent 20ed756 commit ef8915f
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/good-pigs-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] respect fetch cache option
1 change: 1 addition & 0 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ export function create_client({ target, base }) {
uses.url = true;
}),
async fetch(resource, init) {
/** @type {URL | string} */
let requested;

if (resource instanceof Request) {
Expand Down
20 changes: 10 additions & 10 deletions packages/kit/src/runtime/client/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ if (import.meta.env.DEV) {
const method = input instanceof Request ? input.method : init?.method || 'GET';

if (method !== 'GET') {
const url = new URL(input instanceof Request ? input.url : input.toString(), document.baseURI)
.href;
cache.delete(url);
cache.delete(build_selector(input));
}

return native_fetch(input, init);
Expand All @@ -53,9 +51,7 @@ if (import.meta.env.DEV) {
const method = input instanceof Request ? input.method : init?.method || 'GET';

if (method !== 'GET') {
const url = new URL(input instanceof Request ? input.url : input.toString(), document.baseURI)
.href;
cache.delete(url);
cache.delete(build_selector(input));
}

return native_fetch(input, init);
Expand All @@ -67,7 +63,7 @@ const cache = new Map();
/**
* Should be called on the initial run of load functions that hydrate the page.
* Saves any requests with cache-control max-age to the cache.
* @param {RequestInfo | URL} resource
* @param {URL | string} resource
* @param {RequestInit} [opts]
*/
export function initial_fetch(resource, opts) {
Expand All @@ -88,7 +84,7 @@ export function initial_fetch(resource, opts) {

/**
* Tries to get the response from the cache, if max-age allows it, else does a fetch.
* @param {RequestInfo | URL} resource
* @param {URL | string} resource
* @param {string} resolved
* @param {RequestInit} [opts]
*/
Expand All @@ -97,7 +93,11 @@ export function subsequent_fetch(resource, resolved, opts) {
const selector = build_selector(resource, opts);
const cached = cache.get(selector);
if (cached) {
if (performance.now() < cached.ttl) {
// https://developer.mozilla.org/en-US/docs/Web/API/Request/cache#value
if (
performance.now() < cached.ttl &&
['default', 'force-cache', 'only-if-cached', undefined].includes(opts?.cache)
) {
return new Response(cached.body, cached.init);
}

Expand All @@ -110,7 +110,7 @@ export function subsequent_fetch(resource, resolved, opts) {

/**
* Build the cache key for a given request
* @param {RequestInfo | URL} resource
* @param {URL | RequestInfo} resource
* @param {RequestInit} [opts]
*/
function build_selector(resource, opts) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
let force = false;

export function _force_next_fetch() {
force = true;
}

/** @type {import('./$types').PageLoad} */
export async function load({ fetch }) {
const resp = await fetch('/load/cache-control/count');
const resp = await fetch('/load/cache-control/count', { cache: force ? 'no-cache' : 'default' });
if (force) {
force = false;
}
return resp.json();
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
<script>
import { invalidate } from '$app/navigation';
import { _force_next_fetch } from './+page';
/** @type {import('./$types').PageData} */
export let data;
async function fetch_again() {
/** @param {'default' | 'force' | 'bust'} type */
async function fetch_again(type) {
await fetch('/load/cache-control/increment');
if (type === 'force') {
_force_next_fetch();
} else if (type === 'bust') {
await fetch('/load/cache-control/count', {method: 'POST'});
}
invalidate('/load/cache-control/count');
}
</script>

<p>Count is {data.count}</p>
<button on:click={fetch_again}>Fetch again</button>
<button class="default" on:click={() => fetch_again('default')}>Fetch again</button>
<button class="force" on:click={() => fetch_again('force')}>Fetch again (force reload)</button>
<button class="bust" on:click={() => fetch_again('bust')}>Fetch again (prior POST request)</button>

Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ export function GET({ setHeaders }) {
setHeaders({ 'cache-control': 'public, max-age=4', age: '2' });
return json({ count });
}

export function POST() {
return new Response();
}
48 changes: 36 additions & 12 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -552,24 +552,48 @@ test.describe('Load', () => {
expect(await page.textContent('p')).toBe('This text comes from the server load function');
});

test('load does not call fetch if max-age allows it', async ({ page, request }) => {
await request.get('/load/cache-control/reset');
test.describe.serial('', () => {
test('load does not call fetch if max-age allows it', async ({ page, request }) => {
await request.get('/load/cache-control/reset');

page.addInitScript(`
page.addInitScript(`
window.now = 0;
window.performance.now = () => now;
`);

await page.goto('/load/cache-control');
expect(await page.textContent('p')).toBe('Count is 0');
await page.waitForTimeout(500);
await page.click('button');
await page.waitForTimeout(500);
expect(await page.textContent('p')).toBe('Count is 0');
await page.goto('/load/cache-control');
expect(await page.textContent('p')).toBe('Count is 0');
await page.waitForTimeout(500);
await page.click('button.default');
await page.waitForTimeout(500);
expect(await page.textContent('p')).toBe('Count is 0');

await page.evaluate(() => (window.now = 2500));
await page.click('button');
await expect(page.locator('p')).toHaveText('Count is 2');
await page.evaluate(() => (window.now = 2500));
await page.click('button.default');
await expect(page.locator('p')).toHaveText('Count is 2');
});

test('load does ignore ttl if fetch cache options says so', async ({ page, request }) => {
await request.get('/load/cache-control/reset');

await page.goto('/load/cache-control');
expect(await page.textContent('p')).toBe('Count is 0');
await page.waitForTimeout(500);
await page.click('button.force');
await page.waitForTimeout(500);
expect(await page.textContent('p')).toBe('Count is 1');
});

test('load busts cache if non-GET request to resource is made', async ({ page, request }) => {
await request.get('/load/cache-control/reset');

await page.goto('/load/cache-control');
expect(await page.textContent('p')).toBe('Count is 0');
await page.waitForTimeout(500);
await page.click('button.bust');
await page.waitForTimeout(500);
expect(await page.textContent('p')).toBe('Count is 1');
});
});

test('__data.json has cache-control: private, no-store', async ({ page, clicknav }) => {
Expand Down

0 comments on commit ef8915f

Please sign in to comment.