relayStylePagination not working when after or before are not in root args

[Nightbr]

Hey folks, I ran into an issue with relayStylePagination and nestjs-query because in NestJs query all pagination params are in paging object.

See more here.

Intended outcome:

When calling fetchMore the next results should append to the existing set of data.

Actual outcome:

When calling fetchMore, the next results override the existing set of data.

How to reproduce the issue:

Here is my InMemoryCache configuration:

 new InMemoryCache({
      typePolicies: {
        Query: {
          fields: {
            myObjects: relayStylePagination(),
          },
        },
      },
    });

Here is my page component:

export const MyObjectsPage = () => {
  const { loading, error, data, fetchMore } = useMyObjectsQuery({
    variables: {
      paging: {
        first: 2,
      },
    },
  });

  if (loading) return <div>loading...</div>;
  if (error) return <div>Error! {error.message}</div>;

  const nodes = data.myObjects.edges.map((edge) => edge.node);
  const pageInfo = data.myObjects.pageInfo;

  return (
    <>
      <Table variant="simple">
        <Thead>
          <Tr>
            <Th>Status</Th>
            <Th>Name</Th>
          </Tr>
        </Thead>
        <Tbody>
          {nodes.map((node) => (
            <Tr key={node.id}>
              <Td>{node.status}</Td>
              <Td>{node.name}</Td>
            </Tr>
          ))}
        </Tbody>
      </Table>
      {pageInfo.hasNextPage && (
        <Button
          onClick={() => {
            fetchMore({
              variables: {
                paging: {
                  first: 2,
                  after: pageInfo.endCursor,
                },
              },
            });
          }}
        >
          Fetch more
        </Button>
      )}
    </>
  );
};

You can see that in fetchMore, pagination params are in a paging object.

fetchMore({
  variables: {
    paging: {
      first: 2,
      after: pageInfo.endCursor,
    },
  },
});

And if we look at the relayStylePagination code:

https://github.com/apollographql/apollo-client/blob/master/src/utilities/policies/pagination.ts#L93

if (args.after) {
  const index = prefix.findIndex(edge => edge.cursor === args.after);
  if (index >= 0) {
    prefix = prefix.slice(0, index + 1);
    // suffix = []; // already true
  }
} else if (args.before) {
  const index = prefix.findIndex(edge => edge.cursor === args.before);
  suffix = index < 0 ? prefix : prefix.slice(index);
  prefix = [];
} else {
  // If we have neither args.after nor args.before, the incoming
  // edges cannot be spliced into the existing edges, so they must
  // replace the existing edges. See #6592 for a motivating example.
  prefix = [];
}

We can see it gets args after & before only if they are at root of args (which is not the case). So we go to the else and it replace the existing edges.

Here is another comment which talk about this issue -> https://github.com/apollographql/apollo-client/issues/6502#issuecomment-869053815

Possible solutions

1. Override relayStylePagination merge method to flatten paging object at root and pass args.after & args.before
2. Maybe we could pass a settings object to relayStylePagination with the path to the paging object (after, before) by default it's args root.

Versions

  System:
    OS: Linux 5.4 Linux Mint 20.2 (Uma)
  Binaries:
    Node: 12.21.0 - ~/.nvm/versions/node/v12.21.0/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.14.11 - ~/.nvm/versions/node/v12.21.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.107
    Firefox: 90.0
  npmPackages:
    @apollo/client: ^3.3.21 => 3.3.21 
    apollo-server-express: 2.25.2 => 2.25.2 

Thanks for your help, feel free to ask anything and if you're ok with a solution, I can make a PR :ok_hand:

Cheers!

[benjamn]

@Nightbr Can you share the (relevant parts of the) query you're using? I'm wondering how exactly you're consuming the variables.paging variable when you use the Query.myObjects field in your query, because those field arguments are what the relayStylePagination policy ends up seeing.

In case this is helpful, it looks like your Apollo Client URLs are still referring to the master branch—we switched to main last August, around the time of @apollo/client@3.1.4 (though what's published to npm shouldn't be affected by branch names). I mention this mostly because the relayStylePagination helper has seen a number of improvements since then.

Override relayStylePagination merge method to flatten paging object at root and pass args.after & args.before

This is always an option, either by starting with relayStylePagination() and wrapping its read and merge methods, or by copy/pasting the relayStylePagination code and reimplementing how it uses args. Nothing wrong with that! The pagination helpers are primarily intended as inspiration for your own policy helpers, not necessarily for direct reuse (especially if you're going off the beaten path, which you seem to be doing here).

[Nightbr]

@benjamn Hey thanks for your complete answer!!

I got the latest code from main branch, thanks for pointing this :ok_hand: -> https://github.com/apollographql/apollo-client/blob/main/src/utilities/policies/pagination.ts

Here is my gql

query myObjectsQuery($filter: MyObjectFilter, $paging: CursorPaging, $sorting: [MyObjectSort!]) {
  myObjects(filter: $filter, paging: $paging, sorting: $sorting) {
    edges {
      cursor
      node {
        createdAt
        id
        name
        status
        updatedAt
        workspaceId
      }
    }
    pageInfo {
      endCursor
      hasNextPage
      hasPreviousPage
      startCursor
    }
  }
}

and CursorPaging type:

export type CursorPaging = {
  /** Paginate after opaque cursor */
  after?: Maybe<Scalars['ConnectionCursor']>;
  /** Paginate before opaque cursor */
  before?: Maybe<Scalars['ConnectionCursor']>;
  /** Paginate first */
  first?: Maybe<Scalars['Int']>;
  /** Paginate last */
  last?: Maybe<Scalars['Int']>;
};

Everything else respects the Relay pagination convention.

But yes, like you said, I have Copy/Paste the relayStylePagination and handle my own version. Perhaps I will create a lib with some utilities with this and the settings to configure where to find after & before args.

[zanechua]

This used to work and I think the updates broke that unsupported? functionality.

Ran into this exact issue today where the pagination arguments were inside a variable and the relayStylePagination function wouldn't work properly.

Is the apollo team open to making a change to the relayStylePagination to accept an additional argument for where the pagination variables are located?

I use apollo-cursor-pagination and the following typeDefs: https://github.com/FleekHQ/apollo-cursor-pagination/blob/master/tests/test-app/src/type-defs/cat.js

With the change of the pagination variables being placed in an input type.

E.g.

"Input for Pagination"
input PaginationInput {
  "Slice from the back"
  first: Int
  "After the cursor"
  after: String
  "Slice from the front"
  last: Int
  "Before the cursor"
  before: String
  "Order by column"
  orderBy: String
  "Order by direction"
  orderDirection: OrderDirection
  "Multiple order by directions"
  orderDirectionMultiple: [OrderDirection]
  "Multiple order by columns"
  orderByMultiple: [String!]
  "Whether to use Offset Pagination"
  useOffsetPagination: Boolean
}

If I didn't do this, I'll have to be introducing duplicate pagination args for every single query that I want paginated.

e.g.

offersByUserConnection(input: PaginationInput): OffersConnection!
offersConnection(input: PaginationInput): OffersConnection!
allOffersConnection(input: PaginationInput): OffersConnection!

would become

offersByUserConnection(first: Int, after: String, last: Int, before: String, orderBy: String, orderDirection: OrderDirection, orderDirectionMultiple: [OrderDirection], orderByMultiple: [String!], useOffsetPagination: Boolean)
offersConnection(first: Int, after: String, last: Int, before: String, orderBy: String, orderDirection: OrderDirection, orderDirectionMultiple: [OrderDirection], orderByMultiple: [String!], useOffsetPagination: Boolean)
allOffersConnection(first: Int, after: String, last: Int, before: String, orderBy: String, orderDirection: OrderDirection, orderDirectionMultiple: [OrderDirection], orderByMultiple: [String!], useOffsetPagination: Boolean)

From what I've read, there doesn't seem to be a way to define a set of arguments and reuse them other than the above approach.

Thoughts?

I solved the issue by just modifying the lines with patch-package:

https://github.com/apollographql/apollo-client/blob/9a1b816b268085df0ba7de534262c17cd715e0d6/src/utilities/policies/pagination.ts#L190-L194

https://github.com/apollographql/apollo-client/blob/9a1b816b268085df0ba7de534262c17cd715e0d6/src/utilities/policies/pagination.ts#L199-L200

to

if (args && args.after || args && args.input && args.input.after) {
    const after = args.after || args.input.after;
    var index = prefix.findIndex(function (edge) { return edge.cursor === after; });

if (args && args.before || args && args.input && args.input.before) {
    const before = args.before || args.input.before;
    var index = prefix.findIndex(function (edge) { return edge.cursor === before; });

[zanechua]

Since this still hasn't gotten a response from the apollo team, utilities.cjs.native.js was added and I had to patch that file too.

This is my patch-package file for @apollo/client:3.6.8

@apollo+client+3.6.8.patch

diff --git a/node_modules/@apollo/client/utilities/policies/pagination.js b/node_modules/@apollo/client/utilities/policies/pagination.js
index bdf7b9f..76cb888 100644
--- a/node_modules/@apollo/client/utilities/policies/pagination.js
+++ b/node_modules/@apollo/client/utilities/policies/pagination.js
@@ -99,14 +99,16 @@ export function relayStylePagination(keyArgs) {
             }
             var prefix = existing.edges;
             var suffix = [];
-            if (args && args.after) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.after; });
+            if (args && args.after || args && args.input && args.input.after) {
+                const after = args.after || args.input.after;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === after; });
                 if (index >= 0) {
                     prefix = prefix.slice(0, index + 1);
                 }
             }
-            else if (args && args.before) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.before; });
+            else if (args && args.before || args && args.input && args.input.before) {
+                const before = args.before || args.input.before;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === before; });
                 suffix = index < 0 ? prefix : prefix.slice(index);
                 prefix = [];
             }
diff --git a/node_modules/@apollo/client/utilities/utilities.cjs b/node_modules/@apollo/client/utilities/utilities.cjs
index 817f777..3b6cde8 100644
--- a/node_modules/@apollo/client/utilities/utilities.cjs
+++ b/node_modules/@apollo/client/utilities/utilities.cjs
@@ -847,14 +847,16 @@ function relayStylePagination(keyArgs) {
             }
             var prefix = existing.edges;
             var suffix = [];
-            if (args && args.after) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.after; });
+            if (args && args.after || args && args.input && args.input.after) {
+                const after = args.after || args.input.after;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === after; });
                 if (index >= 0) {
                     prefix = prefix.slice(0, index + 1);
                 }
             }
-            else if (args && args.before) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.before; });
+            else if (args && args.before || args && args.input && args.input.before) {
+                const before = args.before || args.input.before;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === before; });
                 suffix = index < 0 ? prefix : prefix.slice(index);
                 prefix = [];
             }
diff --git a/node_modules/@apollo/client/utilities/utilities.cjs.native.js b/node_modules/@apollo/client/utilities/utilities.cjs.native.js
index 817f777..3b6cde8 100644
--- a/node_modules/@apollo/client/utilities/utilities.cjs.native.js
+++ b/node_modules/@apollo/client/utilities/utilities.cjs.native.js
@@ -847,14 +847,16 @@ function relayStylePagination(keyArgs) {
             }
             var prefix = existing.edges;
             var suffix = [];
-            if (args && args.after) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.after; });
+            if (args && args.after || args && args.input && args.input.after) {
+                const after = args.after || args.input.after;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === after; });
                 if (index >= 0) {
                     prefix = prefix.slice(0, index + 1);
                 }
             }
-            else if (args && args.before) {
-                var index = prefix.findIndex(function (edge) { return edge.cursor === args.before; });
+            else if (args && args.before || args && args.input && args.input.before) {
+                const before = args.before || args.input.before;
+                var index = prefix.findIndex(function (edge) { return edge.cursor === before; });
                 suffix = index < 0 ? prefix : prefix.slice(index);
                 prefix = [];
             }

[moritzvonhase]

Is there any updates on this one?