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.

Sign up for GitHub

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

Clarify active query limitation for refetchQueries #11357

Merged
merged 3 commits into from
Nov 20, 2023
Merged

Clarify active query limitation for refetchQueries #11357

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a12cdaf
Clarify active query limitation for refetchQueries
nopelluhh Nov 9, 2023
441b135
Update phrasing/position
nopelluhh Nov 13, 2023
5f21a3d
Merge branch 'main' into clarify_refetch_documentation_for_active_que…
jerelmiller Nov 20, 2023
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
2 changes: 2 additions & 0 deletions docs/source/data/mutations.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ You can provide the `refetchQueries` option either to `useMutation` or to the mu

Note that in an app with tens or hundreds of different queries, it can be challenging to determine exactly which queries to refetch after a particular mutation.

> Only active queries can be refetched using this option. If the query you want to refetch is not used by a component on the current page, you may want to consider modifying your mutation to support updating the cache directly as described below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To save other folks from trying to refetch inactive queries, it would be best to place this callout right at the top of the section, under the heading ## Refetching queries. I would also rewrite it to be explicit about the definition of "active queries."

Suggested change
> Only active queries can be refetched using this option. If the query you want to refetch is not used by a component on the current page, you may want to consider modifying your mutation to support updating the cache directly as described below.
> You can only refetch _active_ queries. Active queries are those used by components on the current page. If the data you want to update is not fetched by a component on the current page, it's best to [update your cache directly](#updating-the-cache-directly).

@jerelmiller , would you be able to verify that this is an accurate definition for "active queries"? Like @nopelluhh , I found it used on this page but without a clear definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @Meschreiber - I've updated the wording to your suggestion and moved up directly under the code example. Happy to move all the way to the top if you'd prefer, but I was wary of introducing a potential limitation before explaining the API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay on this, a lot of the team has been at a conferences as of late.
This placement breaks up the code sample and the explanation of the code sample, so I would prefer to bump it back down, or back up, depending on how relevant it is to readers. I agree that it's best to place potential limitations lower, but am inclined to put more absolute disqualifies higher up.

@jerelmiller , when you get a chance, would you be able to confirm the definition of "active queries" (those used by components on the current page?) and whether refetchQueries can only be used by such queries? That'll help us figure out where this disclaimer belongs. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nopelluhh!! So sorry I've been so slow to respond and review. I was speaking at 2 conferences this week so I've let GitHub reviews slip through the cracks a bit. I have this on my list to review for accuracy next week. Thanks for the patience!

Copy link

@pfurmaniak pfurmaniak Nov 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey guys, this is a really good change as I've spent too much time than I'd care to admit figuring this one out.

@Meschreiber @jerelmiller is there a possibility to implement a functionality that will refetch inactive queries as soon as they become active so that this behavior is more straightforward? Because of lack of this feature I've implemented my own mechanism to do so, but I bet I'm not the only one who'd think that should be the default behavior.

To clarify: I appreciate the fact that in a big, enterprise-grade application it will be very hard to determine which queries need refeching. However, in smaller applications this kind of behavior would streamline the development massively - I think. Let me know your opinion.

@nopelluhh Great PR anyway - thanks a lot.

Many thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point re: breaking up the example @Meschreiber, will wait for @jerelmiller's review before moving around again. And no prob on the delay @jerelmiller, the docs will survive another day. I'm OOO next week so take your time/feel free to adjust as you see fit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfurmaniak definitely an interesting suggestion! Would you mind opening a feature request in our feature requests repo so we can track that separately? No guarantee we will add/implement something exactly like that, but it gives us the ability to ensure we don't forget that type of use case when we revisit refetchQueries in the future. Thanks!


## Updating the cache directly

### Include modified objects in mutation responses
Expand Down