[AC3] FieldPolicy merge function with external variables cache issues

[offirgolan]

I have the following search query as well as a custom field policy for the search query with a merge function to handle fetchMore pagination. When any of the args listed in the keyArgs changes, the cache "resets" and the hook requests the first page of data with the new args.

The issue here is that I also need to have hasFooIds and fooIds to have the same effect as the other arguments added to keyArgs. Unfortunately, these arguments aren't passed directly to the search query so I can't add them to the keyArgs.

Search Query

query MY_SEARCH_QUERY(
  $index: String!
  $query: String
  $filter: String
  $page: Int!
  $perPage: Int!
  $ranking: [String!]
  $hasFooIds: Boolean = false
  $fooIds: [String]
) {
  search(
    index: $index
    query: $query
    filter: $filter
    page: $page
    perPage: $perPage
    ranking: $ranking
  ) {
    totalResults
    page
    perPage
    results {
      id
      name
      description

      foos(ids: $fooIds) @include(if: $hasFooIds) {
        id
        name
      }
    }
  }
}

Field Policy with merge function

{
  typePolicies: {
    Query: {
      fields: {
        search: {
          keyArgs: ['index', 'query', 'filter', 'ranking', 'perPage'],
          merge(existing, incoming, { args }) {
            if (!existing) {
              return incoming;
            }

            const results = [...existing.results];
            const start = args
              ? (args.page - 1) * args.perPage
              : results.length;
            const end = start + incoming.results.length;

            for (let i = start; i < end; ++i) {
              results[i] = incoming.results[i - start];
            }

            return { ...existing, ...incoming, results };
          }
        }
      }
    }
  }
};

useQuery Options

{
  fetchPolicy: 'network-only',
  nextFetchPolicy: 'cache-first',
  notifyOnNetworkStatusChange: true
}

Intended outcome:

Changing fooIds which is not part of keyArgs

1. Page 1 is requested: merge(undefined, page1)
2. Page 2 is requested: merge(page1, page2)
3. variables.fooIds changes
4. Page 1 is requested: merge(undefined, page1)
5. Page 2 is requested: merge(page1, page2)

Changing filter which is part of keyArgs

1. Page 1 is requested: merge(undefined, page1)
2. Page 2 is requested: merge(page1, page2)
3. variables.filter changes
4. Page 1 is requested: merge(undefined, page1)
5. Page 2 is requested: merge(page1, page2)

Actual outcome:

Changing fooIds which is not part of keyArgs 👎

1. Page 1 is requested: merge(undefined, page1)
2. Page 2 is requested: merge(page1, page2)
3. variables.fooIds changes
4. Page 1 is requested: merge(oldPage2, page1)
5. Page 3 is requested: merge(oldPage2 + page1, page3)

Changing filter which is part of keyArgs 👍

1. Page 1 is requested: merge(undefined, page1)
2. Page 2 is requested: merge(page1, page2)
3. variables.filter changes
4. Page 1 is requested: merge(undefined, page1)
5. Page 2 is requested: merge(page1, page2)

How to reproduce the issue:

Versions
ApolloClient: v3.2.4
System:
OS: macOS 10.15.6
Binaries:
Node: 10.18.1
Yarn: 1.22.4
npm: 6.13.4
Browsers:
Chrome: 86.0.4240.80
Firefox: 72.0.2
Safari: 14.0

[benjamn]

@offirgolan It sounds like you're overusing keyArgs. Any argument you put in keyArgs becomes part of a unique key for the field value (appended to the name of the field in the internal cache data). When the value of a key argument changes, you get a different field key, meaning field values with different key arguments are totally separate in the cache.

This is a safe default policy, since the cache doesn't know what any of your arguments mean, so it has to assume every possible combination of key arguments is logically distinct (unless you tell it otherwise), but it's not a great policy when you want to reuse existing cache data with slightly different arguments.

Guessing from your argument names, I suspect keyArgs: ["query"] is all you need. The reason query makes sense for keyArgs is that it keeps search results completely separate for different query values, so you don't have to examine args.query in your merge and read functions. You could store/retrieve search results by query in your merge and read functions, and use keyArgs: false to disable the argument-based field key behavior, but that would probably complicate the internal representation of the data.

All other arguments can be examined at runtime in merge and read functions via options.args. You probably want to handle an argument like filter in a read function, rather than putting it in keyArgs, because you might use multiple args.filter values when reading the same data, so you definitely do not want differences in args.filter to result in separate field values.

Here's a draft of some new documentation that should help visualize what happens to your keyArgs within the cache: #7175

[offirgolan]

Hi @benjamn, thank you for your response and the link to the updated docs but this doesn't really solve the issue at hand. I understand how keyArgs affect the cache and from my testing, the pagination and merge function behave as expected with the keyArgs I've specified.

The issue is when variables that aren't part of the keyArgs change, the new page is being merged with an existing previous page where instead, existing should be undefined. I have no way to tell the cache to give a new set of results for when (in this example) fooIds changes.

[offirgolan]

All other arguments can be examined at runtime in merge and read functions via options.args. You probably want to handle an argument like filter in a read function, rather than putting it in keyArgs, because you might use multiple args.filter values when reading the same data, so you definitely do not want differences in args.filter to result in separate field values.

I'm not really sure I understand what you mean by this. How would examining filter in the merge and read functions work? In my case filter, query, ranking, and index are all sent to an elastic search based service to produce a new set of paginated search results.

There is also no reference to the previous args so there is nothing to compare them to...

[benjamn]

@offirgolan My suggestion about examining args in your read and merge functions was based on my assumption (possibly wrong!) that you'd like to be able to use multiple different filter values in your client application, without making additional network requests. If you don't mind making a request for each different combination of arguments, then it does make sense to keep arguments like filter in keyArgs, to preserve the default behavior of caching field values separately by their arguments. That's certainly easier than implementing a client-side version of the filter logic, which is what you'd need to do to handle more queries/variables directly from the cache.

That said, I now realize this issue is about a much narrower question: how to make sure the $hasFooIds variable is reflected in this field key system. Since $hasFooIds is passed as an argument to a directive, it's not technically a field argument, though it does behave like a field argument, in the sense that it affects the presence or absence of the field value.

One immediate answer to this question is that keyArgs can be a function, and that function has access to the full set of variables, so you could include variables.hasFooIds in your field key:

new InMemoryCache({
  typePolicies: {
    Result: { // guessing this __typename
      fields: {
        foos: {
          keyArgs({ fooIds }, { variables: { hasFooIds } }) {
            return JSON.stringify({ fooIds, hasFooIds });
          },
        },
      },
    },
   },
})

I've configured this keyArgs function for the Result.foos field, since that's where $hasFooIds is consumed, but I suppose you could write a similar function for the SearchResults.results field (assuming the Query.search field returns an object of type SearchResults). Since the variables are the same throughout the query, I believe either Result.foos or SearchResults.results could take responsibility for including variables.hasFooIds in its field key.

Is that closer to answering your question?

[offirgolan]

I think we're getting closer but I don't believe this is quite it.

Currently the pagination works as expected when any of the args listed in the search keyArgs change (filter, query, ranking, and index). The main issue is that I want the same behavior to occur when fooIds changes as well (which is a valid argument passed to the foos query).

In the example you've given above, you're handling hasFooIds which is used only in the @include directive but fooIds should already be part of the foos keyArgs by default.

Also, would the nested keyArgs of foos have any affect on the merge function declared for the search query?

[benjamn]

@offirgolan I'm out of guesses, but I would like to continue the discussion here, so I think a simplified reproduction would really help.

Also, would the nested keyArgs of foos have any affect on the merge function declared for the search query?

From the perspective of the merge function for the Query.search field, the keyArgs for Result.foos should only affect the foos field names within each Result object. Specifically, those field names will look like foos:{"fooIds":[...],"hasFooIds":...} instead of just foos, and there may be multiple such keys (because of the different argument values).

Since your merge function is currently not concerned with the internal format of those Result objects, it probably doesn't need to worry about the keyArgs for Result.foos, because it can handle the Result objects as opaque data.

I recommend setting a breakpoint in your merge function to see what I mean—and of course let me know if you see anything there that doesn't make sense.

[hwillson]

Hi @offirgolan - regarding your original comment:

Unfortunately, these arguments aren't passed directly to the search query so I can't add them to the keyArgs.

If you write a custom keyArgs function like this, you should be able to leverage the variables that aren't passed into the query, like fooIds:

const cache = new InMemoryCache({
  typePolicies: {
    Query: {
      fields: {
        search: {
          keyArgs(
            { index, query, filter, ranking, perPage },
            { variables: { fooIds } }
          ) {
            return JSON.stringify({
              index,
              query,
              filter,
              ranking,
              perPage,
              fooIds,
            });
          },
          // ... everything else
        },
      },
    },
  },
});

This keyArgs will translate into the following when data is stored in the cache:

This use of a keyArgs function should align the functionality differences you were seeing, with regards to merge function existing data being undefined when updating filter, but returning a previous page when updating fooIds.

[hwillson]

Let us know if this is still a concern with @apollo/client@latest - thanks!