apollo-client: fetchMore and fetchPolicy: cache-and-network does not work together

See also #6327 and #6833, especially this comment.

Looks like the proper policy combination in 2021 for paging+cache is

    fetchPolicy: 'cache-and-network',
    nextFetchPolicy: 'cache-first',

After thinking more about this, I agree with @Akryum the extra network fetch after fetchMore is not a reasonable consequence of using network-only or cache-and-network fetch policies.

Yes, you may be able to work around the problem with nextFetchPolicy: "cache-first", but I no longer think using nextFetchPolicy should be necessary in this case. As long as the cache read triggered by fetchMore is complete (in the diff.complete sense), I think we should deliver that cache result immediately, and skip the network. In other words, reacting to a fetchMore request is not one of the situations where we want/need to reapply the original query’s fetchPolicy literally, so I think it should be safe to ignore the fetchPolicy here, and just deliver the result.

I’m still investigating the consequences of these changes, but I think/hope we can remove the this.reobserve() from the finally after fetchMore, and change the QueryInfo code as follows:

commit 5c378d1d07a33ab4de0c14d2f6d68d48297ef9ec
Author: Ben Newman <ben@apollographql.com>
Date:   Tue Aug 17 16:00:56 2021 -0400

    Avoid full reobservation in QueryInfo listener when diff.complete.

diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts
index ea23f374a..7f5b6ce5d 100644
--- a/src/core/QueryInfo.ts
+++ b/src/core/QueryInfo.ts
@@ -235,7 +235,8 @@ export class QueryInfo {
         // full reobservation, since oq.reobserve might make a network
         // request, and we don't want to trigger network requests for
         // optimistic updates.
-        if (this.getDiff().fromOptimisticTransaction) {
+        const diff = this.getDiff();
+        if (diff.complete || diff.fromOptimisticTransaction) {
           oq["observe"]();
         } else {
           oq.reobserve();

The observe method of ObservableQuery delivers the current result only, whereas reobserve reapplies the fetchPolicy, potentially delivering multiple results.

I hope to have more answers soon. Thanks to @akryum for sharing your findings!

Some good news, at long last: using @Akryum’s reproduction and @apollo/client@3.6.0-beta.10, I see no extra network requests after the fetchMore calls (thanks to #9504). These changes have just landed (today) on the release-3.6 branch, so we are actively looking for help testing them.

While this change seems like the right behavior for most use cases, there’s a chance it will be disruptive for code that was previously relying on fetchMore triggering extra network requests (hard to imagine, but certainly possible). If you’re reading this, please help us test/validate these changes by updating your projects with npm i @apollo/client@beta when you have the chance! Your help will get v3.6 released sooner (in April).

Reproduction: https://codesandbox.io/s/apolloclient-beta-fetchmore-bug-forked-8mplu?file=/src/App.js

Click multiple times on the load more button:

Indeed apollo client does a “phantom” request after the fetch more one:

I had a similar issue to @zakton5. I originally though it was a fetchPolicy issue, but what was actually causing the behavior was a custom merge function for a field that originally looked like

merge: (existing, incoming) => incoming || existing

That field could be null for some of my data while I was doing a fetchMore, so existing=undefined and incoming=null. My merge function would return undefined, which must’ve triggered some silent error causing apollo to retry the original query, with the network status going from 3 -> 1.

Changing my merge to be:

merge: (existing, incoming) => {
  if (!incoming && existing) {
    return existing;
  }
  return incoming;
}

caused the issue to stop happening.

what is the solution at the moment ? still have this issue with @apollo/client": "^3.5.8, I have disabled local cache.

 const { loading, error, data, fetchMore} = useQuery(GET_CUSTOMER_TICKETS, {
    variables: {
      first: DEFAULT_PAGE_SIZE,
      last: DEFAULT_PAGE_SIZE
    },
    fetchPolicy: 'no-cache',
    nextFetchPolicy: 'no-cache',
  });

I can also confirm this behavior, but I can’t use the workaround. I really need cache-and-network to trigger as soon as my variables change (every time the variables change). It seems the fetchPolicy + nextFetchPolicy doesn’t really do it for me 😕 Is this planned at all? seems like it’s been more than a year

I’ve just spent ~3.5h debugging my code, then Apollo, investigating what appears to be the same issue as described, in a lazy loading list view.

Fetch more is correctly fetching and merging the next page of data, but a second request without the pagination token was then being sent and replacing the data in merge, causing the page content to reset and reset the scrollbar to the end, tiggering a refetch of the second page again, repeating infinitely

@khitrenovich suggestion to set

    fetchPolicy: 'cache-and-network',
    nextFetchPolicy: 'cache-first',

appears to suppress the second call and things now seem to work as expected.

What’s frustrating is this query is only used in one place, so I’m not sure what exactly is subscribed to be notified which could be triggering this behavior.

If I remove the reobserve from fetchMore, there is another one called as a side-effect of this writeQuery: https://github.com/apollographql/apollo-client/blob/7856c187ebf89df80c446834ab238e5475e6114d/src/core/ObservableQuery.ts#L372

This writeQuery call will lead to setDiff being called as well, but more importantly notify: https://github.com/apollographql/apollo-client/blob/7856c187ebf89df80c446834ab238e5475e6114d/src/core/QueryInfo.ts#L205

This then calls a reobserve too, causing a new request with the initial variables/options to be sent: https://github.com/apollographql/apollo-client/blob/7856c187ebf89df80c446834ab238e5475e6114d/src/core/QueryInfo.ts#L233

Then the result from fetchMore is reverted by the new fetch.

Having the same issue in Apollo Client 3.3.13 Using fetchPolicy: 'network-only' and notifyOnNetworkStatusChange: true, calling fetchMore will trigger an unwanted additionnal network fetch which will mess up with the type policy merge.

I can’t use nexFetchPolicy since I’m using network-only. Please send help 🏳️

Digging into the source code…

The unwanted fetch seems to be caused by this line: https://github.com/apollographql/apollo-client/blob/336337dbedc108f13035f85eacf5dde1b47ddfd8/src/core/ObservableQuery.ts#L333 Which in turn calls this with the initial variables/options: https://github.com/apollographql/apollo-client/blob/336337dbedc108f13035f85eacf5dde1b47ddfd8/src/core/Reobserver.ts#L52

On 3.4 the lines are all in ObservableQuery.ts but the logic is the same: https://github.com/apollographql/apollo-client/blob/7856c187ebf89df80c446834ab238e5475e6114d/src/core/ObservableQuery.ts#L383 And the fetch: https://github.com/apollographql/apollo-client/blob/7856c187ebf89df80c446834ab238e5475e6114d/src/core/ObservableQuery.ts#L658

Not sure what a concast is though.

@alex-golubtsov I believe you’re describing a related issue to the one mentioned here -> https://github.com/apollographql/apollo-client/issues/6907

Addition from myself: The same issue goes for the network-only policy. The only one that is working properly is cache-first. useQuery / useLazyQuery -> same effect.

For any poor soul who couldn’t figure this out like me. My problem was unique in that I had set custom keyFields for the specific type I was querying for.

keyFields: ["id", "target", ["entityId"]]

The problem for me specifically was that target could sometimes be null. I think Apollo got upset when this happened because it was trying to access target.entityId and couldn’t. So maybe there was an internal error (no logs about it) and it just retried the query with the default variables/arguments?

My fix was to just remove entityId entirely. target was sufficient.

keyFields: ["id", "target"]

Any update? At this moment I can’t use fetch more or create an infinite scroll 😭.

There are some workaround to make this fetchMore?

So what’s the verdict here? Is it reasonable to expect/say this is a valid solution and no harm comes from it - hence it’d make it onto the next release ❤️? (Wishful thinking - I know, but hey, what’s there if there’s no hope?)

There is probably an architectural issue if a notify() causes a full new fetch?

I tried again on the codesandbox with 3.4.0-rc.23. The issue is kinda still here on the network side: the second phantom/unnecessary request with first: 1 is still made, but it doesn’t call the merge (so it doesn’t break pagination). So the bug halfway-fixed! 🔧

Any update on people trying to fix this???