apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.28k stars 2.64k forks source link

awaitRefetchQueries to wait for refetch of queries affected by cache eviction after a mutation #11909

Open eltonio450 opened 2 weeks ago

eltonio450 commented 2 weeks ago

Proposal for Enhanced Cache Management Post-Mutation for Better UI Consistency

Context

Currently, our cache update mechanism after mutations aims to keep data up-to-date, but it requires significant maintenance and often leads to inconsistencies in the UI. Here’s a breakdown of the current methods and their drawbacks:

  1. Guessing Modified Fields

    • Method: Automatically update the cache when mutation results include guessed fields.
    • Issues: Selecting the correct fields is challenging and maintenance-heavy. Reusability of the code suffers due to the need for specific fragments.
      mutation sendMessage(...) {
      id
      parentTopic {
       id
       # Uncertainty about which fields to fetch
      }
      }
  2. Specific refetchQuery

    • Method: Use refetchQueries with specific query names.
    • Drawbacks: Identification of the "parent" query is difficult, especially when multiple queries might be dependent on the component's location.
      {
      "refetchQueries": ["myTopicQuery"],
      "awaitRefetchQueries": true
      }
  3. Broad Refetch Queries

    • Method: Use a broad refetch policy like "all" or "active".
    • Cons: This method is costly in terms of bandwidth and number of queries executed.
      {
      "refetchQueries": "all",
      "awaitRefetchQueries": true
      }
  4. Evicting Cache Objects

    • Method: Directly evict an object from the cache.
    • Cons: Does not integrate well with awaitRefetchQueries; leads to UI inconsistencies as updates may happen seconds after success messages are displayed.
      cache.evict("Topic:"+myParentTopicId);

Proposal

Implement a new cache management mechanism that combines eviction and refetching. After a successful mutation, the system should:

This solution aims to minimize the number of refetches (reducing bandwidth and load) while ensuring that no necessary updates are missed.

Request for Feedback

I am looking for feedback on this approach or suggestions if there are existing methods to achieve this with the current system configuration that may have been overlooked.

Thank you for considering this enhancement.

dylanwulf commented 2 weeks ago

I've tried utilizing method 4 in my app, but the main problem I ran into was that evicting the data from cache causes that data to be immediately removed from the screen while the refetch is in progress. This causes an annoying flashing effect which imo is not a good user experience. Just out of curiosity, is this something you ran into as well? Were you able to do anything about it?

eltonio450 commented 2 weeks ago

I think the last versions handle some kind of optimistic mechanism that allows to evict without this flash, but not 100% sure

jerelmiller commented 2 weeks ago

Hey @eltonio450 👋

Thanks for the issue! I think you've done a good job laying out the options and I don't really have other ideas on how to approach this. Your pros/cons laid out are pretty much the same list I would have come up with myself.


I've been thinking about this a lot lately and I'd love to get a bit better system in place in a future version. In my opinion, that refetchQueries API is a bit rough and is a bit unintuitive (mostly because it relies on "active" queries which seems to be a misunderstood concept).

I'd love to add an invalidate API similar to evict that would cause queries affected by the invalidated data to refetch. Unlike evict though, this would not remove data from the cache. I believe this would be a bit more foolproof since it would not require knowledge of which queries use that data, but you still get the benefit of a network refetch to refresh it (granted at the extreme end, this could suffer a similar problem as `refetchQueries: 'all' where it could cause a cascade of many network requests).

We currently have an INVALIDATE sentinel that you can use with cache.modify, but unfortunately it only causes read functions to be run again and doesn't execute a refetch on its own. I'm unsure if we should repurpose this sentinel object (which would need to be done in a major), or whether introducing a cache.invalidate(...) API in a minor would make more sense (the downside being that "invalidate" would mean two different things depending on which API you're using which might cause confusion).

There is nothing in the works for this right now and its not something that is currently prioritized. I wanted to add this comment here saying that we've been thinking about it and agree its something the client needs. Ideally we'll do an RFC process for this feature so we can gather community feedback to make sure we're thinking about this right. Your proposal though aligns with this so I'm glad to hear there's some validation on the need for this.

eltonio450 commented 2 weeks ago

thank you for the feedback

evict is indeed a bit rough, the api naming could be more "declarative" (evict is really imperative/you have to be really knowledgable about the cache to use it), in the sense that the developper wants to declare: "ok, I know that objects that will be affected are ..." and the magic happens (and it can!) something like:

sideEffects: [{__typename: "Topic", id: "32"}]
jerelmiller commented 2 weeks ago

evict is indeed a bit rough, the api naming could be more "declarative" (evict is really imperative/you have to be really knowledgable about the cache to use it)

I think the intention is that evict is imperative since its meant to give you control over when something should be removed from the cache, either because its not needed anymore, or because you need to free up some space.

That being said, we can certainly explore a more declarative API that might help invalidate certain records after a mutation, though I can't promise anything. Certainly something for us to be aware of!