apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Revisit `refetchQueries` API #419

Open jerelmiller opened 10 months ago

jerelmiller commented 10 months ago

The refetchQueries API used primarily to refetch data after a mutation is a bit unintuitive in its current form. We've had quite a few questions about this feature where people misunderstand how it works.

The key part of this API primarily misunderstood is that refetchQueries only refetches active queries, which are queries that are currently mounted on screen. Once a query has been unmounted/unsubscribed, it will no longer be refetched. This presents a lot of confusion when devs think a bug exists because they don't see the network request after the mutation.

As such, I'd like to revisit this API in future major and see what we can do to improve this to make the behavior more intuitive.

thclark commented 1 month ago

Honestly just better documentation on it would be the useful thing - perhaps an example showing an off-screen/unmounted and an on-screen/mounted container. Revalidate them both, but see the network request fire only for the second?

Because the behaviour as-is is technically a feature, right? Why refetch data for things that aren't shown? Or is it that unmounted queries are not marked as invalid (ie when re-mounted they're not also refetched) which would understandably cause friction (because you'd always have to refresh on remount)?

Perhaps there should be an option to refetchQueries that's like strategy=ACTIVE_ONLY|ACTIVE_LAZY|OPTIMISTIC where

I don't know enough about the fetching mechanisms to understand if a fourth option OPTIMISTIC_DELAYED makes sense (in which active queries are refreshed first, then inactive queries get refreshed) - perhaps this could offer a performance advantage over OPTIMISTIC if a lot of large-but-inactive queries exist?

jerelmiller commented 2 weeks ago

@thclark I do agree that documentation could be improved, though long-term I'd like to get away from the document-based thinking here if at all possible. There are too many footguns with the existing API that make it difficult to do the right thing. For example, its too easy to add a new query somewhere in your app that might be changed by a mutation and forget to add that query to the list of "refetch queries" for that mutation.

The other issue is that the way we currently store values in Apollo Client internals doesn't allow for us to do anything with "inactive" queries. Inactive queries are actually removed from the internal data store once they become inactive, which then become garbage collected. To allow for something like a strategy: OPTIMISTIC, we'd need an entirely new way of storing those inactive queries so that we have a reference to what they are later. This could introduce memory leaks though if we aren't careful because we'd need to come up with a new way to determine when we can actually allow those inactive queries to get garbage collected in a way that make sense.

What I'd like to pursue instead is some kind of field-based invalidation API where rather than using queries themselves to force refetches, you can mark a field in the cache as stale which would force any queries dependent on that field to be refetched. If a query is active at that time, it would be refetched immediately. If the query is "inactive", then we'd refetch that query as soon as the query mounts. I believe this would scale a bit better than our current solution. Long-term I'd love for this to replace refetchQueries entirely, but definitely not something we'd do until we have validation that this type of API is actually a suitable replacement.

Regardless, even if we keep refetchQueries long-term, I'd like to experiment with it to see if there is any way to make it more intuitive than its current form to try and avoid some of the footguns we have today.

Sorry for the brain dump, but hope this helps describe where my head is at!