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

invalidate: callback not called for server load, invalidate never reloads server endpoints #6780

Closed
aloker opened this issue Sep 13, 2022 · 15 comments
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@aloker
Copy link
Contributor

aloker commented Sep 13, 2022

Describe the bug

I'm not really sure if this is intentional or a bug, but anyway:

invalidateAll correctly triggers all load functions including calling the server side equivalents (+layout.server.ts, +page.server.ts).

Accordingly, I would expect the callback passed to invalidate to be called with some URL that represents the server side load functions to selectively trigger a refetch of those resources. However, it is only called for URLs used in explicit fetch or depends calls in load functions. As a consequence, invalidate( () => true) does not behave like invalidateAll. I realize it's not documented to behave the same, but intuitively I'd expect it to.

As a corollary, there is no URL that can be passed to invalidate (the non-callback version) that would reload the server side endpoints. The only way to trigger server side page/layout endpoints to reload is by using invalidateAll.

Reproduction

See https://stackblitz.com/edit/sveltejs-kit-template-default-zrzasd?terminal=dev

The page shows the timestamp of the data coming from different layers (server side layout load, layout load, server side page load, page load).

The button "Test invalidate" invokes invalidate and collects all URLs passed to the callback (note that the callback always returns false - we do not actually need to refresh anything for this test). The page shows the list of URLs that were passed to invalidate.

Note that the explicit dependencies "app://layout-load" and "app://page-load" show up in the list, but nothing more.

"Test invalidate with refresh" does the same but returns true from the callback. This causes the two load functions to run, but does NOT invoke the server side endpoints to be called. As a result, the server side data is missing from $page.data.

"Invalidate all" calls invalidateAll which refreshes all data as expected.

Logs

No response

System Info

(Stackblitz)
  System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.73 
    @sveltejs/kit: next => 1.0.0-next.481 
    svelte: ^3.46.0 => 3.50.1 
    vite: ^3.1.0 => 3.1.0

Severity

serious, but I can work around it

Additional Information

No response

@aloker
Copy link
Contributor Author

aloker commented Sep 13, 2022

This is probably related to #6459

@dummdidumm
Copy link
Member

Not sure what's the best label for this / the best way to procede. We definitely should document it better though that invalidateAll invalidates all load functions, regardless of client and server, while invalidate(() => true) will only invalidate those load functions which have at least one dependency (i.e. the load fetch was used or depends was called, both only possible for the isomorphic load currently)

@dummdidumm dummdidumm added the documentation Improvements or additions to documentation label Sep 14, 2022
@aloker
Copy link
Contributor Author

aloker commented Sep 14, 2022

Not sure what's the best label for this / the best way to procede. We definitely should document it better though that invalidateAll invalidates all load functions, regardless of client and server, while invalidate(() => true) will only invalidate those load functions which have at least one dependency (i.e. the load fetch was used or depends was called, both only possible for the isomorphic load currently)

I think it would be quite useful to be able to selectively re-fetch the page/layout endpoints (ie. each node) with invalidate. E.g., if I know that only the page data needs refreshing but none of the layout server data, I could call invalidate('the-url-of-the-page-server-load'). I'm not sure what the right naming schemes for those URLs would be (maybe something like /the/route/+page.server, /+layout.server etc). Likewise the callback in invalidate(callback) should be called with each node's url. That would mean that invalidate(()=>true) has the same effect as invalidateAll(), which is intuitively what I'd expect.

@dummdidumm
Copy link
Member

invalidate is called with each node's URL, but right now +page/layout.server don't have the fetch and/or depends function available to it which would make this work. I believe there's a related feature request for that.

@Rich-Harris
Copy link
Member

Rich-Harris commented Sep 20, 2022

This is intended behaviour. If you need to invalidate a specific load function (server or shared) you can use depends with a custom identifier. This is already documented in the depends and invalidate docs, I'm not sure that there's more we can do?

@Rich-Harris Rich-Harris added this to the whenever milestone Sep 20, 2022
@aloker
Copy link
Contributor Author

aloker commented Sep 20, 2022

This is intended behaviour. If you need to invalidate a specific load function (server or shared) you can use depends with a custom identifier. This is already documented in the depends and invalidate docs, I'm not sure that there's more we can do?

Thanks for the feedback. I wasn't aware that I can use depends in the server load function (I somehow assumed it only existed in the isomorphic version). This solves the issue I'm trying to solve.

Frankly, I still find it a bit counter-intuitive that invalidate(()=>true) != invalidateAll(). But you are right that it works as documented. I'll close this issue.

@aloker aloker closed this as completed Sep 20, 2022
@bitcrshr
Copy link

bitcrshr commented Sep 20, 2022

I wasn't aware that I can use depends in the server load function (I somehow assumed it only existed in the isomorphic version). This solves the issue I'm trying to solve.

Unless I'm mistaken, depends isn't available on the server load fn (in +page.server.ts). Is the implication here that invalidating something that the load in +page.ts depends on will also trigger the load fn in +page.server.ts to rerun?

Honestly, the dependencies/invalidation system was pretty mysterious to me before, and it's even more so now. Not blasting the changes at all, just hoping that this can be made more clear 😁

I stumbled here because I'm having issues with this myself. Prior to migration, I had a route that had a GET endpoint that fed the initial data, a load fn that listed a dependency campaigns, and a call to invalidate('campaigns') once a modal closed.

Migrated the GET to a load fn in +page.server.ts and put the load fn in +page.ts with the necessary changes, but it just doesn't work anymore. I'm trying to figure out if there's a bug in my logic or a bug in the framework lol, so any clarification here would be greatly appreciated.

@aloker
Copy link
Contributor Author

aloker commented Sep 20, 2022

I wasn't aware that I can use depends in the server load function (I somehow assumed it only existed in the isomorphic version). This solves the issue I'm trying to solve.

Unless I'm mistaken, depends isn't available on the server load fn (in +page.server.ts). Is the implication here that invalidating something that the load in +page.ts depends on will also trigger the load fn in +page.server.ts to rerun?

That's what I thought at first, too, but I fired up a blitzstack project and was able to use depends in +page.server's load function, eg depends('app://test') Edit: I originally wrote here "The server side load (and only the server side load, not the isomorphic one) was triggered when I called invalidate('app://test').", but in fact triggering the server side load always triggers the isomorphic (shared) load as well.

The server side load is not triggered when a dependency of the isomorphic load is triggered.

@bitcrshr
Copy link

Interesting! This is good to know, and makes sense now--thanks! But if that's the case, what mechanisms do we have for retriggering the server load? Is it only invalidateAll()? I have to think there is a less clunky way to refresh the page data without triggering everything else to reload.

@aloker
Copy link
Contributor Author

aloker commented Sep 20, 2022

Interesting! This is good to know, and makes sense now--thanks! But if that's the case, what mechanisms do we have for retriggering the server load? Is it only invalidateAll()? I have to think there is a less clunky way to refresh the page data without triggering everything else to reload.

As I wrote, in your server load, just call depends with a custom url (a generic url such as api://server or a more specific one such as api://users/all?server - you can invent a scheme that makes sense to you) and invalidate that url - any load call that depends on the invalidated (custom) url will retrigger.

@bitcrshr
Copy link

Hmm, I think there may have been some wires crossed here--let me try to get on the same page!

When I use "server load", I'm referring to the load fn inside of +page.server.ts, which does not have a depends passed into it. This is what used to be a GET endpoint.

The load fn that has depends available to it is in +page.ts, which I believe is what "isomorphic load" has been referring to here.

Taking a look at the StackBlitz you posted, the only time that PageServerLoad gets updated / logged is on first page load, and then whenever the Invalidate all button is clicked--the load fn in +page.server.ts does not get called any other time.

I guess on some level I understand why it works this way, but I'm still wondering if invalidateAll() is really the only way to get the PageServerLoad fn in +page.server.ts to re-run after first page load.

@aloker
Copy link
Contributor Author

aloker commented Sep 20, 2022

The stackblitz in my original post does not reflect what I said in my later posts.

Here's an example that shows different scenarios for invalidate: https://stackblitz.com/edit/sveltekit-selective-invalidation?file=src%2Froutes%2F%2Bpage.svelte

(Note that I wrote earlier that triggering a server side load would not trigger the shared/isomorphic load if both are present, but that's not true. The shared load - if present - is always called when the server side load is called - which makes sense, because the server side data always has to pass through the shared load function and is not automatically merged into PageData.)

BTW: yes "isomorphic load" refers to +page.ts or +layout.ts whereas "server load" refers to +page.server.ts and +layout.server.ts.

BUT: I just realized that the typescript types are not complete - depends is not defined in ServerLoadEvent but is actually passed in (and works, see the stackblitz). I'll file an issue.

@bitcrshr
Copy link

Ah, now it makes sense! Apologies for all the going around, and thanks for explaining!

@aloker
Copy link
Contributor Author

aloker commented Sep 20, 2022

Filed the bug as #6929

@aloker
Copy link
Contributor Author

aloker commented Sep 21, 2022

The missing depends function is now fixed

https://github.com/sveltejs/kit/releases/tag/%40sveltejs/kit%401.0.0-next.492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants