-
Notifications
You must be signed in to change notification settings - Fork 64
feat(query-db-collection): support automatic refetch on query invalidation #381
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
base: main
Are you sure you want to change the base?
Conversation
|
@@ -834,7 +834,7 @@ describe(`QueryCollection`, () => { | |||
|
|||
// Wait for refetch to complete | |||
await vi.waitFor(() => { | |||
expect(queryFn).toHaveBeenCalledTimes(2) | |||
expect(queryFn).toHaveBeenCalledTimes(3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test might need to be changed, open to feedback!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh wait... did we already have support for invalidating queries?? 😅 It looks like we just added another implementation... it seems Query was already handling this internally... if this is the case, then we have a lot less to do, just update the docs/jsdocs a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely appears so, and to be honest, that's what I've been confused about. Still not completely following the need for refetch in general, if under the hood we're reading from the cache from query? But I don't have full context as well. 😄
I'll go ahead and remove the changes.
If this behavior is handled by Query already, do we want to mention that directly in docs/query-collection.md? What did you have in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah when building this originally and writing this issue it didn't occur to me that the query client would just keep calling the queryFn like normal.
Refetch is nice as not everyone is migrating from a full blown normal Query setup so they might just have a one-off collection that they want to be able to manually refetch. It depends I suppose on your mental model of your state. A web of data or standalone table-like collections.
There's a few places where we mention calling other collections to refetch (search for "refetch". So we should mention first that you can invalidate a tag or just directly tell those collections to refetch.
The build is failing — also let's update docs/jsdocs where we recommend calling refetch on collections to also mention invalidate as an option. |
From #344:
Adds support for automatic refetching of collections when queryClient.invalidateQueries is called with a matching queryKey. This removes the need for manually coordinating utils.refetch() calls across related collections.
Tests are passing, but I’d appreciate a second look to confirm they cover the right scenarios — especially around observed vs. passive (unobserved) invalidation.