Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch more with merge policy existing data comes undefined or previous incoming data with duplicates. #6729

Closed
ErkinKurt opened this issue Jul 29, 2020 · 8 comments

Comments

@ErkinKurt
Copy link

Intended outcome:
I'm not sure that I totally understood the merge vs updateQuery but here is the case:
When I use merge field policy and fetch more in the same time, I expected to see that data populates with new ones.

Actual outcome:
I have the people array as I do the first request via useQuery and the data populates with the expected data. However when I do fetchMore, here is what happens:
I do fetch more, new data comes as incoming, however my existing data is undefined which is I think odd because I already have the data and showing in the UI. As you can see:

image
image

If I do fetchMore one more time, this time the existing will be filled with the previous incoming data but not the one that is already showing. And it will concatenate the prev with the incoming and the duplicate data will populate forever. But there will be no change in the UI at all.
image

When I provide read field policy it will fetchMore as expected and I can see existing data in merge but then when the filterType changes, I can't even get the new data and only read is triggered with the existing data. By using updateQuery inside fetchMore I don't see any of the issues and it works as expected.

How to reproduce the issue:

Reporduce repo:
https://github.com/ErkinKurt/react-apollo-error-template

You can change status type and it will change the data that's shown but when you press load more you will get nothing. As I tried to explain above, you can uncomment read field policy and see what happens. Thanks for your hard work.

Versions

System:
    OS: Windows 10 10.0.18363
  Binaries:
    Node: 12.16.2
    Yarn: 1.22.4 
    npm: 6.14.4
  Browsers:
    Chrome: 84.0.4147.105
    Edge: Spartan (44.18362.449.0)
@benjamn
Copy link
Member

benjamn commented Jul 29, 2020

You're very close to the solution, but there's one more piece that's worth understanding.

The cache can't make any assumptions about the meaning of your field arguments (filterType and pageNumber), so by default it stores data received for the people field separately whenever the arguments are different. Since your fetchMore query has different argument values than the original query, those two people results get stored separately, which is why existing is undefined in the merge function—no data exists for those new arguments.

While this is reasonable default strategy (in the absence of better information), it's often not a very good strategy, especially since fetchMore is intended to grow the original array, rather than storing new results separately. For cases like this, you can control which arguments the cache considers relevant for storage using the keyArgs configuration. Options include keyArgs: ["filterType"], keyArgs: ["pageNumber"], keyArgs: ["filterType", "pageNumber"] (this is the default behavior), and keyArgs: false.

Since you probably want there to be only one array, regardless of the arguments, the most appropriate setting is keyArgs: false, which tells the cache to store the people data using a property like people: [...], rather than something like 'people:({"filterType":...,"pageNumber":...})': [...]. This ensures there's only ever one array, and it works because your merge (and read) functions have a chance to examine options.args to interpret the arguments however you like. If you wanted to keep separate lists according to filterType, you could use keyArgs: ["filterType"] instead.

One additional subtlety: when you define both a merge function and read function for a field, the cache assumes those functions will take responsibility for interpreting the arguments, so keyArgs: false is assumed by default (though you can override it explicitly). For more background on this behavior, see #5680 and #5862. That's why you noticed (correctly) that adding a simple read function fixed the problem. However, if you want to have only a merge function, you need to specify keyArgs: false yourself. Sorry this policy wasn't better documented!

For comparison, our concatPagination helper exported by @apollo/client/utilities uses a keyArgs configuration that defaults to false:

export function concatPagination<T = Reference>(
  keyArgs: KeyArgs = false,
): FieldPolicy<T[]> {
  return {
    keyArgs,
    merge(existing, incoming) {
      return existing ? [
        ...existing,
        ...incoming,
      ] : incoming;
    },
  };
}

If you wanted to use this helper, your code could look like this:

import { concatPagination } from "@apollo/client/utilities"

const cache = new InMemoryCache({
  typePolicies: {
    Query: {
      fields: {
        filterId: {
          read() {
            return filterIdVar();
          }
        },
        people: concatPagination(),
      }
    }
  }
})

However, even with a helper function available, I think it's useful to understand what's going on behind the scenes.

As a final note, it may be tempting to define a read function to go along with your merge function, but we've found that read functions are often unnecessary when using fetchMore, and can complicate things because you have to update the variables for the original query to match the new longer list. With no read function and keyArgs: false, the list just keeps growing, and you don't have to worry about updating the original query.

I know that's a whole lot of information, but thanks for reading, and thanks for providing a reproduction that made it easy to see exactly what was going on!

@ErkinKurt
Copy link
Author

ErkinKurt commented Jul 29, 2020

Perfect explanation, thank you! For the future comers, I recommend this video which helped me to digest the explanation and give more insight about ApolloClient v3.

@joebernard
Copy link
Contributor

@benjamn In your explanation above you mention usage of keyFields instead of keyArgs. Is that a typo?

@benjamn
Copy link
Member

benjamn commented Aug 6, 2020

@joebernard Yes, that was a typo. In this case, I should have only been talking about keyArgs. Thanks for pointing that out!

@TanBeige
Copy link

TanBeige commented Sep 9, 2020

This thread has solved a lot of problems for me, but one is still left open.

What if we have 2 different queries that call on the same field (ex: All Users and Only Known Users) where one field calls a fetchMore (All Users) and the other doesn't (Known Users, this just calls users with a different sort). In my case, using KayArgs=false makes it so that it returns the All Users query correctly (with a fetchMore) but it gets called for every query that uses the field users. So whenever I want to view Known Users it brings back the query All Users instead. Removing KeyArgs should fix the problem, both queries show up, except the fetchMore doesn't concatenate a list because my merge function's existing variable returns as undefined (just like in this thread). What can I do to be able to call 2 separate queries as well as a fetchMore for only one of them?

Edit: I was able to fix this problem by making the non-fetchMore query's fetchPolicy="no-cache". But still, is there a way to fix the problem and cache it as well?

@valeriagrazzini
Copy link

@TanBeige I have the same problem. Same query called with and without fetchmore

@yairopro
Copy link

yairopro commented Mar 13, 2022

For those who will pass by this issue, even after defining the keyArgs I still got the existing = undefined problem.

My graphQL request was like

	query ReservationsList($id: Int!, $offset: Int = 0) {
		user(id: $id) {
			name
			reservations(
				where: {
					#...
				},
				order_by: {time: asc}, 
				limit: 30, 
				offset: $offset
			) {
				id
				time
				numberOfPlaces
			}
		}
	}

It's tricky because here my mistake was that the user parent object didn't include its own id field request, so Apollo didn't even merge the fetchMore's user object with the original.

The solution was quite simple to set (still hard to identify):

	query ReservationsList($id: Int!, $offset: Int = 0) {
		user(id: $id) {
			id  # <- set the id to help Apollo identify the parent object of the list
			name
			reservations(
				#...
			) {
				#...
			}
		}
	}

@defel
Copy link

defel commented Jan 5, 2023

After "debugging" my code for the last 2 hours, and read the very well written docs this way, installed the apollo-client-devtools, tried out various variantes of merge and read and wondering why always existing is undefined, while there are data in the cache (the data is there, I can see them) .. make sure that you did not mixed up refetch and fetchMore 🤦

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants