Conversation
Adds a new remote functions cache API. Simple example:
```ts
/// file: src/routes/data.remote.js
import { query, command } from '$app/server';
export const getFastData = query(async () => {
const { cache } = getRequestEvent();
cache('100s');
return { data: '...' };
});
export const updateData = command(async () => {
// invalidates getFastData;
// the next time someone requests it, it will be called again
getFastData().invalidate();
});
```
For more info see the updated docs.
This is very WIP, and the adapter part isn't implemented yet (there are a few ways to approach it and we need to agree on the other APIs first). But it workds in dev and preview (public cache is implemented as runtime cache which very likely needs some more hardening).
TODOs/open questions:
- right now `event.cache()` is "last one wins" except for tags which are merged. It probably makes sense to allow one entry for public cache and one for private, and either do "last one wins" or "lowest value wins"
- is `ttl` and `stale` descriptive enough? Should it be `maxAge` and `swr` instead (closer to the web cache nomenclature)?
- how to best integrate this with adapters? either they provide a file with some exports which are like hooks which we call at specific points (`setHeaders`, `invalidate` etc) or we don't do anything and do this purely via headers, and adapters can check these headers and either do runtime cache based on it and/or add cdn cache headers (though maybe they have to clone the response then; not sure how much of an overhead that is and if that matters)
- this only works for remote functions right now, and it only works when you are calling them from the client. We could additionally have a SvelteKit-native runtime cache for public caching, and/or the adapter can hook into this to cache somewhere else than in memory (Vercel can use runtime cache, CF can use their cache, etc; i.e. this is related to the question above). This way we get more cache hits between client/server calls (or rather, we can get full page request cache this way, which we don't have at all right now).
- can this be enhanced in a way that this is usable for full page requests, too (e.g. inside handle hook?). Private cache doesn't make sense there at least. I'd say it's possible to implement and would be intuitive with this API (we can say "do this in handle or load", or "assuming you use remote functions only we take the lowest cache across all of them as the page cache", etc etc, many possibilities) but we should do that later and not bother with it now.
🦋 Changeset detectedLatest commit: 85b8ae4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // shareable across users (CDN caching) or private to user (browser caching); default private | ||
| scope: 'private', | ||
| // used for invalidation, when not given is the URL | ||
| tags: ['my-data'], |
There was a problem hiding this comment.
I don't think the URL is the right cache key here... it should probably be the remote function key instead. Also, tags should be additive, not replace the default key.
|
One thing I don't think is quite right here: Caching remote functions requires two coordinated layers of caching. One of them is a runtime cache at the server level, which needs to be used to cache direct-from-server remote function calls. This cache should be granular -- i.e. if I call Overall, I really think SvelteKit core should not do anything with the information from the cache API -- instead, the adapters should do everything:
|
packages/kit/src/exports/public.d.ts
Outdated
| * Options for [`event.cache`](https://svelte.dev/docs/kit/@sveltejs-kit#RequestEvent) | ||
| */ | ||
| export interface CacheOptions { | ||
| ttl: string | number; |
There was a problem hiding this comment.
Should be a type like
`${number}d` | `${number}h` | `${number}m` | `${number}s` | `${number}ms`There was a problem hiding this comment.
Ms/day don't really make sense imo but otherwise yes; on my list of todos once we agree on the API 👍
|
Would |
Both these points are correct, and in fact right now this PR doesn't do anything in core with the public cache, this is all handled outside of the SvelteKit runtime (dev/preview for now, adapters are todo). We can discuss how exactly to do this once we agree on the user-facing API👍 |
|
Disorganised thoughts incoming:
|
|
A third option, between an import and a property of import { query, command } from '$app/server';
export const getFastData = query(async () => {
- const { cache } = getRequestEvent();
- cache('100s');
+ query.cache('100s');
return { data: '...' };
});This solves all the problems at once — we don't need to worry about cluttering |
|
While we're thinking about a cache API, I'd love for us to think about bfcache — this is one of the very few genuine weaknesses of SPA navigation relative to The Olde Web. Even then, traditional bfcache is a bit ropey. If I click into one of my pending tasks, mark it as complete, and then navigate back to my task list, the completed task is still visible in the pending list. You see this bug a lot with GitHub. But the back navigation sure is fast! We can offer the best of both worlds: we can keep the list of pending tasks cached, but also invalidate it if one gets completed. That way, when I navigate back without completing a task it can happen instantly, but if I did update the list then SvelteKit knows that it has to fetch fresh data. This might be controversial but I think this behaviour should be opt-out rather than in. Something like this, perhaps: export const getPendingTasks = query(async () => {
// disable bfcache altogether for this query
query.bfcache(false);
return await db.select().from(task).where(...);
});export const getPendingTasks = query(async () => {
// enable bfcache, but configure it
query.bfcache({
limit: 5, // evict if we're more than 5 navigations away from having used this query
maxAge: '10m' // evict after 10 minutes whatever happens
});
return await db.select().from(task).where(...);
});
Implementation-wise, we would continue to remove queries from memory when they were offscreen, but instead of discarding their data altogether we would put them in a session-scoped As with the prerender This would only apply during |
|
I like the
I would argue they are very similar with respects to how you use them. The "only" difference is that one cache lives in the user's browser and the other lives in your CDN or server runtime. The web's caching headers also share all the same stuff and you can then declare whether this should be saved in the user's browser or the CDN. The way I think about this feature is "you get caching you know from the web's caching headers but with the possibility to invalidate them before they run out".
It's not in other people's browser storage, it's in your runtime storage or the CDN, and that you can easily invalidate with that.
Just from reading that I wouldn't know what
They are a very good way to organize your API surface into specific categories. But yes, we could start without them, if we make it possible to invalidate all permutations of a query (e.g.
This is different to
To quote you:
Caching/when to refresh in general is a topic we need to discuss further. It's sensible to e.g. want to refresh the data every X seconds proactively if it's used, or reload on each navigation (back or forth), and so on (also see #15039). So I agree with much of what you laid out there. |
Ah so it's not like That in itself might cause some confusion — if I have a response that I know is good to cache for 10 minutes, and will never be invalidated, then I probably want it cached by both CDNs and browsers. I don't want the browser to have to go back to the CDN even for a 304, I already said it was good for 10 minutes. I would guess that the majority of cases are like that (i.e. can be expressed with bog-standard For this reason I still think we need clearer separation at an API level. It's probably fine for a single API to cover both public and private caches if we're just mirroring HTTP semantics, but for stuff that needs to be invalidated (and therefore needs adapter buy-in, and involves often-paid-for additional platform features) it has to be something different. Perhaps this is where it does make sense to use tags after all, precisely because that maps to how platforms think about cache purging, and having tags be something meaningful (rather than generated from remote function keys) is the feature — I want to be able to go into my CDN dashboard and clear stuff, or POST to a webhook, and in both cases I need to be dealing with predictable keys. Something like this perhaps: export const getPost = query(v.string(), async (slug) => {
query.cacheByTag(['post', `post:${slug}`]);
return await db.select().from(post).where(...);
});
export const invalidatePost = command(v.string(), (slug) => {
if (!(await isAdmin())) return;
query.invalidateTag(`post:${slug}`);
});
export const invalidateAllPosts = command(v.string(), (slug) => {
if (!(await isAdmin())) return;
query.invalidateTag('post');
});(Note that I didn't bother to include a
This approach also eliminates the
We absolutely cannot be in the business of guessing what the developer intended when it comes to this stuff; there are meaningful consequences to getting it wrong. Even the most conservative option (ignore
query.cache('60s');
query.cache('60s', { scope: 'public' });
query.cache({ maxAge: '60s' });
query.cache({ maxAge: '60s', scope: 'public' }); |
|
(If we agree on keeping the APIs separate, then we can punt on |
… now, rename args
Adds a new remote functions cache API. Simple example:
For more info see the updated docs.
This is very WIP, and the adapter part isn't implemented yet (there are a few ways to approach it and we need to agree on the other APIs first). But it workds in dev and preview (public cache is implemented as runtime cache which very likely needs some more hardening).
TODOs/open questions:
query.cache()is "last one wins" except for tags which are merged. After sitting with it for a while this is I think the most straightforward and understandeable solution, but we can also approach it differently.ttlandstaledescriptive enough? Should it bemaxAgeandswrinstead (closer to the web cache nomenclature)?setHeaders,invalidateetc) or we don't do anything and do this purely via headers, and adapters can check these headers and either do runtime cache based on it and/or add cdn cache headers (though maybe they have to clone the response then; not sure how much of an overhead that is and if that matters)