[feat] Added more general invalidate with uses(resource)

[icalvin102]

Fixes #1684

Problem

The invalidate function only re-runs load if fetch is used. This is suitable for many case but it does not work with apiClientWrapper libraries or other forms of loading data that do not utilize the fetch option of load.

Solution

Introduce a new uses(resource) function as option of load. uses registers a arbitrary string that can be used as an (resource) identifier in the invalidate function to trigger a re-run of all load's with a matching registered identifier.
invalidate still defaults to reload URLs but if true is given as a second argument a custom resource is invalidated.

Example Usage

<script context="module">
import { api } from '$lib/myapi';
export async function load({page, uses}){
    uses('item-resource');
    const item = await api.getItem(page.params.id);
    return { 
        props: { item }
    }
}
</script>

<script>
import Item from '$lib/Item.svelte';
import { invalidate } from '$app/navigation';
export let item;
</script>

<Item {item} />
<button on:click={()=> invalidate('item-resource', true) }>Reload</button>

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

[changeset-bot]

🦋 Changeset detected

Latest commit: c551006

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Minor

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

[Rich-Harris]

Thank you! Do we need the second argument? I feel like fetch(a) and uses(b) could just add a and b to node.uses.

At the same time I can't help but wonder if this issue reveals a more significant gap. Ideally you'd want api.getItem to call uses on your behalf, since it will know better than its various consumers how to construct the invalidation key. Presumably that's not possible since if it was it could just be wrapping fetch in the first place. Is there a way that the API client could be brought into the SvelteKit lifecycle? Otherwise it can't (for example) access the user's cookies during SSR, and is has to jump through extra hoops to work in both server and client contexts.

[icalvin102]

@Rich-Harris thanks for the review.

I think the second argument (or alternatively a dedicated function to invalidate dependencies that where registered with uses()) is needed because fetch and invalidate force the invalidation key to be a valid URI.
The invalidation key created with uses(resource) is meant to be more generic and should not be constrained by the limitations of a URI.

As renderer.invalid is already implemented as a Set the key could even be any type as long as it is unique.

A generic invalidation key could be used to not only identify a single resource but a group of resources or a hole route/load function as well. This can be helpful in cases where the caller of invalidate() does not know (or is not interested in) the resource URI but rather wants to reload a specific route.

I have no idea how to solve the SSR problem you mentioned but I'm not sure if this is the scope of this PR as invalidate is a client thing. I would also argue that using API clients is more of a SPA thing but ignoring APIWrapper on the server side is of cause no satisfying solution.

[benmccann]

would it be accurate to change it like this?
"any load functions belonging to the currently active page" -> "the load functions belonging to the currently active page and layouts"

[benmccann]

I don't love the two API parameters either. We could create two methods like invalidate and invalidateUrl with the latter prepending location and then calling invalidate. I'm not sure if Rich has better ideas

[Rich-Harris]

I think the second argument ... is needed because fetch and invalidate force the invalidation key to be a valid URI.

They don't. The invalidation key is an arbitrary string that just happens to be a valid URI in the fetch case

[benmccann]

@Rich-Harris I believe he was referring to https://github.com/sveltejs/kit/pull/1709/files#diff-c93946daf844930dace824155fadebf13f4df88ff548cd9b3879606435ebac44R30

[icalvin102]

I think the second argument ... is needed because fetch and invalidate force the invalidation key to be a valid URI.

They don't. The invalidation key is an arbitrary string that just happens to be a valid URI in the fetch case

@Rich-Harris that's true the actual invalidation key is just a string. Sorry that I did not explain further what I meant with "fetch and invalidate force the invalidation key to be a valid URI"

fetch and invalidate convert the incoming string into a URL. This can lead to unexpected behaviour because the resulting invalidation key depends on the location.href.

Example

Consider following example where uses registers the invalidation key the same way fetch does.

We have a routes/__layout.svelte that calls uses('invalidation-key')
and a routes/subroute/index.svelte that calls invalidate('invalidation-key')

If we now navigate to https://xyz/ the invalidation key will be registered as https://xyz/invalidation-key.
The problem is that navigating to https://xyz/subroute/ will cause the invalidation of https://xyz/subroute/invalidation-key which does not exist.

That is the reason why I added the second argument. I don't like the second argument either but without it the invalidate function does not know if it should convert the key to a URL or not.

One solution would be to remove the conversion step from fetch and invalidate and use the untouched argument as the invalidation key but I was not sure what the reason was to implement it like this and if removing would break things.

[ejdrien]

I have a question. Calling invalidate("/") to rerun load in src/routes/index.svelte reruns it only once. All following calls do not rerun load.
But I have built a local @svletejs/kit with these proposed changes and calling invalidate("my-resource", true) reruns the load function every time.

Is this supposed to be happening?

Note: I'm new to Svelte(Kit) and I'm trying to do exactly what this PR allows but I can't seem to do that with the current invalidate method.

[icalvin102]

@ejdrien the original invalidate function is only supposed invalidate api endpoints that where requested with fetch. At least if I understand the code and docs correctly.

[icalvin102]

I removed the URL expansion from fetch and invalidate. This made it possible to get rid of the second argument of invalidate because the invalidation key will be stored unaltered in node.used.dependencies / Renderer.invalid.

[netlify]

✔️ Deploy Preview for kit-demo canceled.

🔨 Explore the source changes: c551006

🔍 Inspect the deploy log: https://app.netlify.com/sites/kit-demo/deploys/61e9d6a3a8490700085076e4

[Rich-Harris]

Finally getting back to this — have updated it such that it's in a mergable state.

I'm still a little iffy about adding this API, beyond the usual reticence about adding API surface area (which is easy to add but incredibly hard to remove) because to me it's indicative of a deeper problem. We don't really want to encourage non-fetch-based API clients because they don't work for SSR beyond trivial cases (and we do want to encourage SSR).

There's a proposal in #2979 that would make it easier to create clients that used fetch under the hood but exposed a more idiomatic API, and I'd be curious to know if that be useful in this case?

If we do end up needing a way to invalidate resources outside of fetch, it occurs to me that the problem described above is solved easily enough by prefixing resources with /. You could do invalidate('/totally-made-up-path') and it would work as expected if you'd declared a dependency on /totally-made-up-path. Might be a bit of a hack, but it would work without adding new API. We would still need a way to declare dependencies, obviously — I think this would perhaps be easier to explain than a uses function:

-export async function load({ params, uses }) {
- uses(`/dependencies/${params.id}`);
+export async function load({ params }) {
  const props = await client.get(params);

  return {
+   dependencies: [`/dependencies/${params.id}`],
    props
  };
}

[icalvin102]

@Rich-Harris thanks for your response.

I really like the idea of returning the dependencies property instead of using the uses function.

I'd imagine the implementation would be as simple as adding the following code to master/packages/kit/src/runtime/client/renderer.js#L683

node.loaded = normalize(loaded);
if (node.loaded.stuff) node.stuff = node.loaded.stuff;
+if (node.loaded.dependencies) {
+    node.loaded.dependencies.forEach((dep) => {
+        const { href } = new URL(dep, url);
+        node.uses.dependencies.add(href);
+    });
+}

Prefixing the custom dependencies with / feels a bit hacky indeed but on the other hand it would provide the benefit of resolving relative paths like fetch does.

I will make a new PR with your proposed solution. This PR can be closed IMO.

Regarding the proposal in #2979 : I would really like this to work but I think it has some major issues in it's current state. I'll comment on that in the issue itself.