apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

Apollo Client 3 reloading query after mutation update

Open danfernand opened this issue 4 years ago • 36 comments

Hello Apollo team. We have an existing app that we are migrating from Apollo Client 2 to 3. We noticed an issue that is affecting our upgrade. After an apollo mutation modifying an entity that previously a query has loaded we are noticing we are setting loading to true if the fetch policy is set to cache-and-network.

We use cache-and-network to guarantee that our app is up to date with the backend and expect the cache to be updated with the network silently.

I am not sure if the behavior is expected since its different than AC 2.

Below I have created 2 code sandboxes that are updating a graphql server created with FaunaDB to show the issue.

Apollo Client 3 - https://codesandbox.io/s/funny-https-zb1hf

Apollo Client 2 - https://codesandbox.io/s/ecstatic-cori-t42ce

When updating completed by selecting the opposite value from a todo the whole page is reloading due to using loading from the query to show a loading state.

Both codesanbox examples are using the same graphql server.

I looking all over to confirm that this AC3 behavior is expected but cannot find anything to that regard.

Thank you for your time.

PS. Great job on the ecosystem! Once we get past this its gonna be awesome!

danfernand avatar Aug 02 '20 00:08 danfernand

experiencing the same issue

hauxir avatar Aug 02 '20 12:08 hauxir

I'm experienced something similar, that might be connected to your behaviour. It feels like an errorneous behaviour of the cache. Your mutation is returning data on the response of the request. The query fetchPolicy is set to cache-and-network. The response of the request is automatically taken to write the data to cache and after that write a refetech is triggered and it should not happen. Rather than that, the cache should be updated and the component just rerender with fresh data and loading state is not set to true.

The core problem is probably the same in my situation. My mutation is not responding with data. Instead of that, I'm just using simple refetchQueries with an array of two query names. The problem here is, that those two queries contain some of the refs of other queries, and those other queries are not part of the refetch array. Those other queries are called within mounted components with queries with fetchPolicy also set to cache-and-network. So what was working in version 2.6.something is, that mutation was called, after it was completed, two network requests have been called, cache has been updated and components were rerendered. The other components, that were using some of the refs of the two queries that were refeteched, were just rerendered with fresh data. No other network requests were made. The main problem is, that right now, instead of 2 queries that should be refetched, i'm having like 5 or more network requests triggered. Some of them even twice :)

kure- avatar Aug 03 '20 17:08 kure-

@danfernand One of the things that changed in AC3 is that cache-and-network really means to read from the cache and make a network request, not just the first time, but whenever there's a cache update that affects this query.

Although I have a specific recommendation to achieve your desired behavior (verified using your reproduction—thanks!), there's a bit of history here that I think is worth understanding. PR #6221 was a big refactoring (released in -beta.46) that made FetchPolicy enforcement more consistent, and restored the -and-network part of cache-and-network, as described above. Realizing that this might not be the behavior folks actually want, we implemented (what we thought was) a clever trick to fall back to cache-first after the first network request, in PR #6353. This behavior was still in place when we released 3.0.0, but post-launch feedback convinced us to revert that PR in #6712, because modifying the options.fetchPolicy that you requested can be surprising and is not universally appropriate. In other words, your reproduction is making unwanted network requests because cache-and-network is being enforced literally.

However, PR #6712 did more than just revert #6353, because cache-and-network-followed-by-cache-first still seems like a very common hybrid policy that people may want to implement. To make that behavior easier to achieve, we also introduced options.nextFetchPolicy (available in @apollo/[email protected]), so you can specify an initial fetch policy (via options.fetchPolicy, as before) and also request a different fetch policy next time.

Long story short, if you use options.nextFetchPolicy as follows, I believe you can get the behavior you're looking for:

export default function Todos() {
  const { loading, error, data } = useQuery(GET_ALL_TODOS, {
    fetchPolicy: "cache-and-network",
    nextFetchPolicy: "cache-first",
    variables: {
      _size: 10
    }
  });
  ...
}

This means the very first query will read from the cache and make a network request, but subsequent queries will make a network request only if the cache data has become incomplete.

benjamn avatar Aug 03 '20 18:08 benjamn

Thank you for your response! Will give nextFetchPolicy a whirl! Apollo has made using graphql amazing. Great job!

danfernand avatar Aug 03 '20 19:08 danfernand

thanks @benjamn , adding nextFetchPolicy seems to be working just well. The behaviour seems to be quite the same as in Apollo version 2 with fetchPolicy: cache-and-network :+1:

kure- avatar Aug 04 '20 10:08 kure-

I added the nextFetchPolicy: "cache-first" to a watchQuery. This watchQuery was updated by a mutation with optimisticResponse & update function. After adding the nextFetchPolicy the eatchQuery doesn't emit the optimistic change anymore.

Did someone experience something similar or can point me in the right direction?

twittwer avatar Aug 11 '20 18:08 twittwer

@benjamn Can you confirm that such behaviour is expected?

Let's say that I have two separate components that run two queries like below:

ComponentA

query A {
  user {
    id
    role
  }
}

ComponentB

query B {
  user {
    id
    role
    name
  }
  products {
    // a lot of fields...
  }
}

When I render ComponentA and run query A, after getting the response user { id, role }, apollo will call also query B because it contains user { id, role }.

AC3 is that cache-and-network really means to read from the cache and make a network request, not just the first time, but whenever there's a cache update that affects this query.

So by a cache update, you meant anything that does something with the cache, right? Running queries, mutations or manually modifying cache by cache.writeFragment, cache.modify, right?

hinok avatar Aug 21 '20 12:08 hinok

We want variable changes to trigger refetch from network, but not changes to the subtree causes by other mutations. With this example, if the mySizeProp changes will it refetch from the network?

  const { loading, error, data } = useQuery(GET_ALL_TODOS, {
    fetchPolicy: "cache-and-network",
    nextFetchPolicy: "cache-first",
    variables: {
      _size: mySizeProp
    }
  });

jperl avatar Sep 02 '20 17:09 jperl

@benjamn I think it would be nice to add this example to the documentation in the section about the update

edbond88 avatar Sep 04 '20 15:09 edbond88

This should really be part of the migration guide 🙏 Went down a lot of rabbit holes to find this issue! https://www.apollographql.com/docs/react/migrating/apollo-client-3-migration/

tim-field avatar Sep 05 '20 06:09 tim-field

I added the nextFetchPolicy: "cache-first" to a watchQuery. This watchQuery was updated by a mutation with optimisticResponse & update function. After adding the nextFetchPolicy the eatchQuery doesn't emit the optimistic change anymore.

Did someone experience something similar or can point me in the right direction?

Can someone say anything about how to handle optimsitic responses?

twittwer avatar Sep 07 '20 07:09 twittwer

We have optimistic responses in our apps and they did not change at all. Two things that I can think of is optimistic payload is incorrect and thus cache is not being updated. There should be an error on the console for this. Another thing is that Apollo client 3 cache cannot cache object if there is no _id or id fields. There would be a console info for this!

danfernand avatar Sep 07 '20 14:09 danfernand

@danfernand The optimistic responses are not changing. The problem I tried to explain is about the active watchQueries. They are not updated after a mutation with optimisticResponse+updateFunction. The watchQuery with fetchPolicy: "cache-and-network" gets updated, but as soon as I add nextFetchPolicy: "cache-first" it doesn't get any update from optimistic mutations.

twittwer avatar Sep 08 '20 08:09 twittwer

I have the following situation:

query A {
  rootField {
     field {
        subfield {
          ...
       }
    }
  }
}

where subfield is an array Mutation that returns none of the rootField or filed or subfield Update function that removes items from subfield array using writeFragment to field

When I call writeFragment the query refetches. I added cache-first and it still refetches. I tried using cache-only but the { data } prop becomes empty, I suppose because Apollo thinks that the cache is incomplete

I would like to only modify the subfield array in the field without causing any extra requests

esseswann avatar Sep 14 '20 15:09 esseswann

I just upgraded from 3.0.2 to 3.2.1 and am experiencing the same issue, for both cache-and-network and network-only fetch policies. In 3.0.2 this issue was not present.

laij84 avatar Sep 25 '20 09:09 laij84

I've been working on a problem similar to this for days. I have an infinite-scoll list, for that reason I use the fetchMore function to incrementally load the required data; in my case this behavior occurs after the fetchMore function is called. The final result is that the number of the queries made to the server is duplicated.

Based on what @benjamn said, I guess that this happens because of the cache update after the new data loaded with fetchMore are merged with the existing ones.

Setting fetchPolicy: "cache-and-network" and nextFetchPolicy: "cache-first" solved the issue.

I definitely agree with @tim-field: this should really be part of the documentation and, in my opinion, the migration guide should have a paragraph to make users aware.

FedeBev avatar Sep 28 '20 09:09 FedeBev

Solved Below issue turned out to be an issue with endCursor and startCursor not updating properly in my custom field policies. 🤕

Original comment I have an issue related to this, but I can't find a nice way of tackling it with fetchPolicy and nextFetchPolicy. My situation is as follows:

I have a list of paginated 'posts' that users can 'like'. This list is ordered by updatedAt. Posts can be updated by any user, so I always need to fetch the latest page of posts to ensure the client is up to date. However, what happens when using the folowing config:

const { data, loading, fetchMore } = usePostsQuery({
    variables: {
      first: pageSize,
      after: undefined,
    },

    fetchPolicy: "cache-and-network",
    nextFetchPolicy: 'cache-first',
    notifyOnNetworkStatusChange: true,
});

Is that any time the user fetches a new page that contained no new posts. That request is cached and requests that follow never fetch new pages. So to solve that I changed it to using cache-and-network for both:

const { data, loading, fetchMore } = usePostsQuery({
    variables: {
      first: pageSize,
      after: undefined,
    },

    fetchPolicy: "cache-and-network",
    nextFetchPolicy: 'cache-and-network',
    notifyOnNetworkStatusChange: true,
});

However, that introduces a new (mostly UX) issue... Namely, whenever a user mutates a post (e.g. through a 'like') the backend returns the updated post and Apollo Client invalidates the cache, causing the pagination query to refetch. The updated post now appears in the new page and then it gets merged to the top of the list. This is normally desired behaviour when fetching the latest posts. However, when a user is scrolled down on the page and the user likes a post, the post dissapears from view (goes to top). I'd like Apollo to mutate it in place.

So in short I'd like to either have Apollo Client not refetch my paginated query when the user mutates and invalidates its query. Or I'd like an option to bypass the nextFetchPolicy when using fetchMore, so that I am certain I will fetch a new page from the backend and not from cache.

Momics avatar Sep 30 '20 08:09 Momics

@Momics, Have you implemented a custom InMemoryCache field resolver for you query? If you haven't, have a look at the documentation about how to customize the cache behavior: with a custom merge function you should be able to keep the order you want of your data. For example you can avoid to invalidate the whole cache and change only the updated objects as desired.

(OT TIP: In my opinion, subscriptions are the best choice for use cases like that)

FedeBev avatar Sep 30 '20 09:09 FedeBev

@FedeBev Thanks for your quick reply!

Indeed I have a field resolver for my query and you made me look at it again which is great. I found that my endCursor and startCursor weren't updating properly, they were both being set to null when getting an empty response from the server. This resulted in the fetchMore function fetching the wrong page. 🤕

Fetchmore is now working properly even with nextFetchPolicy set to cache-first.

And thanks for the tip, I do have subscriptions aswell. I just left them out of the question because they weren't affecting the issue.

Momics avatar Sep 30 '20 09:09 Momics

Same problem here with network-only. Setting nextFetchPolicy to cache-first doesn't solve the problem : it stops from refetching "parent" data, but in fact it stops all refetching, even if variables change. This is a major issue with our migration process to v3...

strblr avatar Dec 16 '20 00:12 strblr

This is definitely a migration blocker. nextFetchPolicy does not solve the issue as it blocks all further updates. I am very worried about all the side effects of nextFetchPolicy.

Currently it is unclear how to achieve the correct (Apollo V2) behavior.

alamothe avatar Jan 01 '21 07:01 alamothe

Seems nextFetchPolicy can work

nyrf avatar Feb 07 '21 14:02 nyrf

Documentation really lacks information about this. There is not even mention about fact, that running one query can trigger another query refetch - . This is very unexpected behavior, can cause infinite loops https://stackblitz.com/edit/apollo-watch-query-infinite-zhiw6w?file=index.js and should be well document with DANGER warnings.

I understood, that fetching entity can update cache and this cache update updates other queries - but without network request. At least this was 2.0 behavior. Now, with this new behavior (which again, should be clearly stated in docs), how can I manually refetch from network when I have to set nextFetchPolicy to cache-first?

jakubnavratil avatar Apr 22 '21 12:04 jakubnavratil

@jakubnavratil I totally agree with you.

I guess the only way to refetch from network is to call the refetch function returned by the useQuery hook. Of course the fetchPolicy should allow network requests, therefore it can't be cache-only

FedeBev avatar Apr 22 '21 12:04 FedeBev

I tested it and manual refetch works ok with nextFetchPolicy: cache-first. I guess lack of documentation and unfortunate name nextFetchPolicy causes lot of misunderstanding.

jakubnavratil avatar Apr 22 '21 12:04 jakubnavratil

At first nextFetchPolicy: catch-first didn't work for me but I found that it was because the fields returned from the mutation didn't match the fields that were specified in the query.

e.g:

query todos {
   todos {
       id
       content
    }
}

mutation addTodo($newTodo: ToDo!) {
   addTodo(todo: $newTodo) {
      content 
   }
}

Because response of addToDo mutation doesn't contain id, apollo client QueryManager sees the cache as 'incomplete' and reverts to a network query.

You can debug this locally by putting a breakpoint on this line: https://github.com/apollographql/apollo-client/blob/f9f8bef1ce1e66c4f7cdc2f163db9f345f41791c/src/core/QueryManager.ts#L1102 and reading the error inside diff.

sinn1 avatar Jun 29 '21 20:06 sinn1

I strongly feel this should be documented. I was trying to get a migration from v2 to v3 working for days when I finally found this thread. I can affirm that using nextFetchPolicy: cache-first works.

You can even have this be the default behavior for your queries unless otherwise specified by including it in the defaultOptions when building your ApolloClient.

export const client = new ApolloClient({
  link,
  cache,
  defaultOptions: {
    watchQuery: {
      fetchPolicy: 'cache-and-network',
      nextFetchPolicy: 'cache-first',
    },
  },
});

This should be in the migration guide to get the same behavior as v2. I'm sure it would help a lot of folks correctly migrate to the latest version.

stangerjm avatar Aug 14 '21 00:08 stangerjm

@stangerjm Your solution almost worked for me, except it only works once.

Here's my situation:

I have a list of members that are either ACTIVE or DEACTIVE. When I activate a member, it immediately updates the UI (thanks to your solution) and everything just works fine. But when I try to activate another member, it doesn't work and it doesn't show me any errors.

Does anyone have any clues here?

chiefGui avatar Aug 27 '21 16:08 chiefGui

idk what's going on behind the scenes but, notifyOnNetworkStatusChange: true, worked for me. i had a UseQuery in the most HOC in the app to bring the data of the connected user.. a refetchQueries in some mutation inside some child component had the network request running + cache updated.. but the UI wasn't rendering cuz onComplete of the UseQuery ( the one mentioned ) wasn't triggered.

Anyone facing same kind of usecase ?

FahmyChaabane avatar Sep 07 '21 18:09 FahmyChaabane

I am having a problem related to what @sinn1 mentioned above

At first nextFetchPolicy: catch-first didn't work for me but I found that it was because the fields returned from the mutation didn't match the fields that were specified in the query.

This seems like a breaking change not mentioned anywhere? Or maybe I'm missing something. But when upgrading to v3, I have a lot of queries being refetched 2, 3, 4 times because a subsequent query fetches only a subset of the data. But I'm fetching data subsets everywhere. Example:

useGetAccount
{
  id
  first_name
  last_name
}
useGetItem
{
  id
  account {
    id
    first_name
  }
}

useGetItem triggers useGetAccount to get fetched again, with "cache-first". only way to solve it is using ""cache-first" + "cache-only" but that's not the behaviour I want when e.g. the variables change these were never re-triggered in v2 Is there anywhere that documents when a query with "cache-first" is refetched? Couldn't find the rules anywhere.

mpgon avatar Sep 08 '21 18:09 mpgon