cache.modify INVALIDATE doesn't result in query refetching data

[vedrani]

Intended outcome:

My motivation is to invalidate data in cache, so that queries can refetch invalidated data.
(For that reason I just return _id from mutation. I know I can return data with mutation, but this scenario has implication on invalidating data on mutations with side effects.)

For that purpose I'm using cache.modify and INVALIDATE flag to invalidate data in cache based on docs.

My expectation is that invalidation will result in query refetching data from server because data is not valid any more.

I also tried evict and it works, but my impression is that invalidate mechanism should allow us to refetch data based on dirty status. Did I miss something in code or did I misunderstood it? Thanks!

Codesandbox

Actual outcome:

Query that fetches all todos is reobserved but it doesn't refetch data.

I tried to debug it and it seems cache diff is non existent. After cache.modify invalidation, query is reobserved but data is resolved from cache instead refetched.

What I found is that INVALIDATE is translated into dirty status here, but Query when calculating diff doesn't see any difference here

How to reproduce the issue:

Open Codesandbox and try to change status of todo item.

It doesn't change.

Versions

System:
OS: macOS 10.15.5
Binaries:
Node: 14.2.0 - ~/.nvm/versions/node/v14.2.0/bin/node
Yarn: 1.22.5 - ~/.yvm/versions/v1.22.5/bin/yarn
npm: 6.14.4 - ~/.nvm/versions/node/v14.2.0/bin/npm
Browsers:
Chrome: 85.0.4183.121
Firefox: 75.0
Safari: 13.1.1
npmPackages:
@apollo/client: 3.2.0 => 3.2.0

[benjamn]

@vedrani You're right, INVALIDATE by itself isn't the best fit with a cache-first policy, since the invalidated data remain in the cache, so the next cache read usually succeeds, so no network request happens.

To make the most of INVALIDATE, you need to have some control over how queries respond to the invalidation signal. If you're doing a mutation, refetchQueries can help, but I am also currently working on a new mutation option that I think can take the place of refetchQueries in most cases:

export default function Todo({ todo }) {
  const [updateTodo] = useMutation(UPDATE_TODO, {
    update(cache, { data: { updateTodo } }) {
      cache.modify({
        id: cache.identify(updateTodo),
        fields(fieldValue, details) {
          return details.INVALIDATE;
        }
      });
    },

    // This callback will be called with each ObservableQuery that was
    // affected by the mutation update function, so you can decide how
    // to refetch the query. The default behavior is the same as calling
    // observableQuery.reobserve(), but calling refetch instead will force
    // a network request.
    onQueryUpdated(observableQuery) {
      return observableQuery.refetch();
    },
  });

I'll let you know when that's ready for testing (or you can watch #7015 for upcoming betas).

Thanks for trying out the INVALIDATE feature in the meantime, and for providing such thoughtful feedback!

[vedrani]

@benjamn I'm also interested are we going to have utility that will allow us to just watch dirty status in cache for some query? Or query callback will be enough?

What is interesting for me is case of expensive or slow query. Will I have opportunity just to show for example Reload button instead of refetching query immediately?

Thanks!

[tafelito]

@benjamn what about queries that are not currently active?

I mean, I have 2 pages, in page 1 I have a list of items, in page 2 I have a function where I can query some data or do something and I need to invalidate data from page 1. Then when I navigate to page 1, I want to show existing data but since cache data is invalidated, I want to refetch data form the server. The fetch policy in page 1 query is cache first

The way I see INVALIDATE or how I would like to use it in this case is whenever I go to page 1, if data is valid and fetchPolicy is cache-first, then no network request is made, but if somewhere if my app I invalidate the data for a particular fieldName, next time, even with cache-first, a network request should be made and cached (invalidated) data should be returned before the request is made.

[stephenhandley]

@benjamn I've tried invalidating a field using "@apollo/client": "3.3.4" and the following based on the above suggestion. thread query still appears to be loading from the cache and doesn't reflect the new post. Shows up fine after reloading the page.

const mutation = useMutation({
  mutation: CreatePost,
  update (cache, {data: {createPost: post}}) {
    const {thread_id} = post;
    cache.modify({
      id: `Thread:${thread_id}`,
      fields: {
        posts: (_, details)=> details.INVALIDATE
      }
    });
  },
  reobserveQuery (query) {
    return query.refetch();
  }
});

similar situation when i try substituing the following

cache.modify({
  id: `Thread:${thread_id}`,
  fields: (_, details)=> details.INVALIDATE
});

I've also tried the following as part of my apollo client config as mentioned here without any luck there either

query: {
  fetchPolicy: 'cache-and-network',
  nextFetchPolicy: 'cache-first'
}

Probably worth adding that this field has a typePolicy for pagination associated with it in case that has some bearing on implementation bugs.

I have to say I've been continually gotcha'd by the over-engineering in Apollo client. The semantics of INVALIDATE should probably be as simple as possible. If I invalidate cache data, no matter what the fetchPolicy is, any subsequent requests to that field (or fields) in the cache should miss and issue a network request.

[justanotherjones]

I have to say I've been continually gotcha'd by the over-engineering in Apollo client. The semantics of INVALIDATE should probably be as simple as possible. If I invalidate cache data, no matter what the fetchPolicy is, any subsequent requests to that field (or fields) in the cache should miss and issue a network request.

Definitely agreed. I spent an unfortunate amount of time today misunderstanding INVALIDATE semantics.

@benjamn If INVALIDATE is not meant to trigger a refetch, what is the correct way to indicate to Apollo client that a specific field is invalid and should trigger a refetch?

[seanemmer]

I have successfully used the DELETE sentinel instead of the INVALIDATE sentinel to force a refetch with a cache-first fetch policy. Not sure if this was the new addition mentioned by @benjamn but it appears to work correctly.

[benjamn]

PR in progress for the reobserveQuery idea I described above: #7827

[omerman]

@benjamn I don't understand how reobserveQuery is a solution to this problem.. I mean, INVALIDATE indicator should refetch, this is why we all came to this thread, it is intuitive that it would work that way, it is expected to trigger a refetch, no matter how the fetch policy is defined. I think I speak for everyone here when I say that we need a way to mark a query as "needs to refetch next time it is observed", outside react & hooks context, using purely the Apollo client.

[ab-pm]

I'm having trouble here deciding between DELETE and INVALIDATE as well. From the original #6991, it seems that INVALIDATE was always meant to trigger re-observation only while DELETE causes a re-fetch due to completely missing data. If not, what else is the difference between them? Do they cause watched queries to return different temporary results while the query is re-run asynchronously?

[omerman]

@ab-pm DELETE means that until data comes back, the data is empty, isn't it? and that is not a good user experience..
In my opinion DELETE should only be used when the resource is deleted.. not for refetch..

[ab-pm]

@omerman Oops you're right. I hadn't noticed the flicker at first because the backend responded so quickly. I had expected that when a query is refetched, the QueryResult would keep its data but loading would switch to true before the new data arrives. But taking a closer look, it's actually the reverse: data becomes null when deleting the cache entries, and during the request loading stays false. That's… puzzling.

Edit: there's prevData though which I had not noticed before - using that as a fallback gives a continuous experience even during refetch.

[jbmikk]

I agree with everyone else here, when I use INVALIDATE I expect Apollo to re-fetch automatically as if the cache was empty.

I expect cache-first to return whatever is in the cache first as long as it is valid content, but not just the first time. Otherwise what's the point?

If cache-first is going work only the first time, then we need a new fetch policy, something like valid-cache-first. And this one should be the client's default policy IMHO.

About the reobserveQuery it's OK to have something like that if someone wants to be very picky about what happens with each query, but I think we need more reasonable defaults, otherwise we always end up wiring everything as if we were not using a framework.

[dylanwulf]

I also agree with all the comments here. I would like a way to do BOTH of these things upon successful completion of a mutation:

• Force all currently mounted queries that depend on a specific field to refetch WITHOUT evicting the field so the query data doesn't flash and annoy the user
• Make sure all unmounted queries that depend on a specific field will fetch from the network when they mount. (it would be okay to not render cached data while this network request is loading.)

The first bullet can be solved by INVALIDATE and onQueryUpdated. The second bullet can be solved with DELETE. But currently there's no way to solve both at the same time as these two solutions interfere with each other.

[yairopro]

So to sum up

1. The current workaround is to use the DELETE sentinel instead of INVALIDATE.
2. Most of us agree that INVALIDATE should invalidate the cache and force a refetch (for cache-first policy at least).
3. A new feature reobserveQuery is born 🐣 from this issue
   Another mouth to feed... I mean, another feature to maintain. Cool

[jbmikk]

Just to add to @dylanwulf 's point and summarize a bit what I think about this:

• We need a cache policy that lets mounted queries show currently cached values (if any) and fetches stale content in parallel (stale meaning flagged with INVALIDATE).
• It should work if the data gets invalidated while a query is being displayed.
• It should work with yet to be mounted queries. This is one reason reobserveQuery is not enough.
• Evicting content is not an option because it causes flashes, we need the stale content while the fetches are in progress.
• Invalidation does not mean a fetch is going to happen: if no query is currently displaying that content and no query ever gets mounted, then it just gets invalidated. Nothing gets fetched until a query actually needs it.
• Perhaps the most important reason reobserveQuery is not enough is coupling: I don't want my mutation code to be aware of all the other queries on the screen. All I know is a specific object or set of objects is not valid anymore, but I have no idea which other components are querying which parts of my schema. I don't want to keep track of that (statically) in the specific code that handles a mutation. Specially considering the other components might change in the future. I just want to let apollo know what's valid and what isn't and let it act accordingly to whatever queries are active at any point in time (dinamically).

EDIT: about the last point, I hadn't noticed the global client.refetchQueries documented here, so it's possible to update all observable queries without having to bother in each mutation. I think I was thinking more about refetch itself when I commented that. I still think it's a bit awkward to have this as the official way of solving the problem, and it still doesn't work with newly added queries in dinamically added components. Having a policy for this just seems the natural way to me and with it we wouldn't need yet another way of manually programming something that's just going to be boilerplate in many cases.