Apollo-client: APC 3: concatPagination() not working as expected

[wittydeveloper]

Hi Apollo team 👋
Congrats for the v3 milestone 🎉

As advised in the v3 warning, we are trying to drop the usage of updateQuery() in favor of TypePolicy merge().
However, we are facing some issues with merge() behavior.

With the following queries, where topicList returns a TopicList type and TopicListItem being an union type on 3 different items types:

query getMainTopicList($completedAfter: DateTime!) {
  activeTopics: topicsList(first: 200, view: ACTIVE) {
    hasAfter
    items {
      ...TopicListItem
    }
  }
  completedTopics: topicsList(first: 25, view: COMPLETED, completedAfter: $completedAfter ) {
    items {
      ...TopicListItem
    }
  }
  flowsList {
    items {
      ...FlowListItem
    }
  }
}
query getCompletedTopicList($after: String) {
  completedTopics: topicsList(first: 50, view: COMPLETED, after: $after) {
    after
    hasAfter
    items {
      ...TopicListItem
    }
  }
}

we use to have the following updateQuery() along with fetchMore() for the getCompletedTopicList query:

updateQuery: (previousResult, { fetchMoreResult }): GetCompletedTopicListQuery => {
  const previousEntry = previousResult ? previousResult.completedTopics : { items: [], __typename: 'TopicsList' };
  const newTopics = fetchMoreResult ? fetchMoreResult.completedTopics.items : [];
  return {
    completedTopics: {
      hasAfter: fetchMoreResult!.completedTopics.hasAfter || false,
      after: fetchMoreResult!.completedTopics.after,
      items: uniqBy([...(newTopics || []), ...previousEntry.items], 'id'),
      __typename: 'TopicsList',
    },
    __typename: 'Query',
  };
},
})

that we remplace with a TypePolicy as it follow:

export const typePolicies: TypePolicies = {
  TopicsList: {
    fields: {
      items: concatPagination(),
    }
  },
}

new pages were now ignored, only the first page remains even when fetchMore() is called
so, we replaced concatPagination() by the following for debug purpose:

export const typePolicies: TypePolicies = {
  TopicsList: {
    fields: {
      items: {
        merge: (existing = [], incoming) => {
          console.log('existing', existing)
          console.log('incoming', incoming)
          return uniqBy([...existing, ...incoming], '__ref')
        }
      }
    }
  },
}

Intended outcome:

new pages fetched using fetchMore({ variables: { after: '...' } }) are added to the results

Actual outcome:

With concatPagination() and custom merge()
new pages are ignored, only the first page remains even when fetchMore() is called

With custom merge()

The following is happening in merge()

1. initial load, existing = [] is an empty array, we return incoming (the first/initial page)
2. when fetchMore() is called, merge() is called with existing = [] and incoming contains the new page

When 2 happens, returning the new page does not update the Query results

• given that topicList is used a many places with different filters and pagination, should we use @connection()?

Versions

  System:
    OS: macOS 10.15.5
  Binaries:
    Node: 10.17.0 - ~/.nvm/versions/node/v10.17.0/bin/node
    Yarn: 1.21.1 - /Volumes/double-hd/assistant.web/node_modules/.bin/yarn
    npm: 6.11.3 - ~/.nvm/versions/node/v10.17.0/bin/npm
  Browsers:
    Chrome: 84.0.4147.89
    Safari: 13.1.1
  npmPackages:
    @apollo/client: ^3.0.2 => 3.0.2 
    apollo: ^2.21.2 => 2.21.2 

[benjamn]

Your original updateQuery function seems to be a bit more complicated than concatPagination, so you're definitely going to need to reproduce the relevant behavior yourself. I recommend taking the helper functions (concatPagination, offsetLimitPagination, etc.) as inspiration, rather than using them directly.

However, since you mentioned the @connection directive, you should be aware that the keyArgs configuration is intended to replace @connection directives entirely. In this case, your aliased fields are distinguished by args.view, so you'll probably want a keyArgs: ["view"] configuration, at the very least, to keep those lists separate in the cache.

This new system is much more powerful than what you had before, but it's important to have an accurate mental model of what's going on. We're happy to answer any questions that come up as you explore it!

[benjamn]

If I'm being very optimistic, there's a _chance_ that specifying keyArgs might do the trick:

export const typePolicies: TypePolicies = {
  TopicsList: {
    fields: {
      items: concatPagination(["view"]),
    }
  },
}

But please don't take my word for it—set some breakpoints and make sure you're following what happens when the field policy functions are executed.

[dylanwulf]

@benjamn If keyArgs are intended to completely replace the @connection directive, then why does the Pagination section in the docs recommend using the @connection directive? Are the docs still a work in progress?

[wittydeveloper]

@benjamn

concatPagination(['view']) did not concatenated new results

I tried the following:

{
  TopicsList: {
    fields: {
      items: {
        keyArgs: ['view'],
        merge: (existing = [], incoming) => {
          console.log('existing', existing[0])
          console.log('incoming', incoming[0])
          return [...existing, ...incoming]
        }
      }
    }
  },
}

initial query (without after variable) works as expected, however,
when after variable is given, merge() behave the following way:

1. receives [] for existing value
2. returns incoming (the new page)

however this is totally ignored since the linked useQuery() that got refreshed without the return of merge()

[benjamn]

@dylanwulf Yes, that specific page in the docs still needs an update. Sorry for the wait!

[benjamn]

@wittydeveloper I think I missed something important earlier: the view and after args are part of the Query.topicsList field, so that's where you need the keyArgs and merge function, rather than the TopicsList.items field. By default, with no keyArgs configuration, the cache assumes all arguments are important, so you get a separate field value for each combination of arguments, which is why the existing data seems to be reset to []. Does that make sense?

[TrevorNovak]

First of all, thanks for all of the work you all doing, I greatly appreciate it.

@benjamn I am experiencing a similar issue as @wittydeveloper, however, my use case is very simple. I am fetching paginated array of data using query params (e.g. page=2) and then simply need to concatenate the next page to the array of existing data.

For instance, the schema for the data being returned looks like

Records {
   records: [Record]
   total: Int,
   pages: Int,
}

Then in new InMemoryCache() I have defined:

typePolicies: {
   Records: {
      fields: {
        records: concatPagination(),
      }
  }
}

I am then feeding this data into a FlatList in React Native, however, the data field is always just the first page of results and existing array is always [] even though incoming appears to always contain the next page of data. I have tried using concatPagination() as well as a merge function.

Any thoughts on what I am doing wrong?

[wittydeveloper]

@wittydeveloper I think I missed something important earlier: the view and after args are part of the Query.topicsList field, so that's where you need the keyArgs and merge function, rather than the TopicsList.items field. By default, with no keyArgs configuration, the cache assumes all arguments are important, so you get a separate field value for each combination of arguments, which is why the existing data seems to be reset to []. Does that make sense?

@benjamn

It works, thanks! 🎉 ✅

I end-up using the following:

Query: {
  fields: {
    topicsList: {
      keyArgs: ['view', 'first'],
      merge: (existing = { __typename: "TopicsList", items: [] }, incoming) => {
        const result = {
          ...incoming,
          items: uniqBy(
            [
              ...existing.items,
              ...incoming.items,
            ],
            '__ref'
          ),
        }
        return result
      }
    }
  }
},

[wittydeveloper]

@benjamn one last curiosity question about merge()

I understood the __ref thing, however, why do I sometimes get object with only __typename and no __ref as below?

They are most the time present in existing and returning them from merge method "freeze" the associated query that gets a undefined data from useQuery()

[MegaCookie]

@wittydeveloper I think I missed something important earlier: the view and after args are part of the Query.topicsList field, so that's where you need the keyArgs and merge function, rather than the TopicsList.items field. By default, with no keyArgs configuration, the cache assumes all arguments are important, so you get a separate field value for each combination of arguments, which is why the existing data seems to be reset to []. Does that make sense?

@benjamn

It works, thanks! tada white_check_mark

I end-up using the following:

Query: {
  fields: {
    topicsList: {
      keyArgs: ['view', 'first'],
      merge: (existing = { __typename: "TopicsList", items: [] }, incoming) => {
        const result = {
          ...incoming,
          items: uniqBy(
            [
              ...existing.items,
              ...incoming.items,
            ],
            '__ref'
          ),
        }
        return result
      }
    }
  }
},

I'm also doing something like this! Thanks! I had to keep in mind that the keyArgs are basically all the input variables of the query that if you change those, you want to completely get rid of all the existing values. So don't include things for the pagination in the keyArgs, but do every other input argument for the query.
Here is an example to merge the elements field for the QueryNamexxx query with the QueryPageable type:

Query: {
  fields: {
    QueryNamexxx: {
      keyArgs: ['arg1', 'arg2'], // Don't include arguments needed for pagination 
      merge: (existing = { __typename: 'QueryPageable', elements: [] }, incoming) => {
        const elements = [...existing.elements, ...incoming.elements].reduce((array, current) => {
          return array.map(i => i.__ref).includes(current.__ref) ? array : [...array, current];
        }, []);
        return {
          ...incoming,
          elements,
        };
      },
    },
  },
},

[wittydeveloper]

Hi @benjamn

I'm following up on my last question regarding some record with only __typename received in merge() args

I understood the __ref thing, however, why do I sometimes get object with only __typename and no __ref as below?

Do you know why this is happening?
Is something missing in our queries or Apollo client configuration?

[agustin-villar]

@wittydeveloper I think I missed something important earlier: the view and after args are part of the Query.topicsList field, so that's where you need the keyArgs and merge function, rather than the TopicsList.items field. By default, with no keyArgs configuration, the cache assumes all arguments are important, so you get a separate field value for each combination of arguments, which is why the existing data seems to be reset to []. Does that make sense?

@benjamn

It works, thanks! 🎉 ✅

I end-up using the following:

Query: {
  fields: {
    topicsList: {
      keyArgs: ['view', 'first'],
      merge: (existing = { __typename: "TopicsList", items: [] }, incoming) => {
        const result = {
          ...incoming,
          items: uniqBy(
            [
              ...existing.items,
              ...incoming.items,
            ],
            '__ref'
          ),
        }
        return result
      }
    }
  }
},

Hi @wittydeveloper I had a similar issue. Based on your solution I tried this and it worked for me:
(Reading the data before merging it did the trick)

Query: {
    keyArgs: ['page'],
    fields: {
        characters: {
            read(data) {
                return data;
            },
            merge(existing = { info: {}, results: [] }, incoming) {
                return {
                    __typename: 'Characters',
                    info: incoming.info,
                    results: [...existing.results, ...incoming.results],
                };
            },
        },
    },
}

[wittydeveloper]

@agustin-villar I tried your read() fix and I still face the issue 😞

[agustin-villar]

@agustin-villar I tried your read() fix and I still face the issue 😞

Hi @wittydeveloper oh, I am sorry, my fix was for the issue where the existing array was empty. While trying to fix my problem I stumble upon custom id implementation for fields, seems to be related to your problem, it might help you: https://www.apollographql.com/docs/react/caching/cache-interaction/#obtaining-an-objects-custom-id

[kirill-konshin]

@gustin-villar read() fix worked for me. Btw, __typename: 'Characters' is not mandatory, it works even without it.

This is quite counter intuitive... no read — no updates, but state after first fetch is correct. Why? Is it a bug?