Skip to content
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

[fix] respect fetch cache option #8024

Merged
merged 2 commits into from
Dec 9, 2022
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/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