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

Add support for cursor-based pagination #8313

Merged
merged 13 commits into from
Feb 20, 2023

Conversation

marekryb
Copy link
Contributor

Currently Keystone supports only offset-based pagination through take and skip arguments in GraphQL queries.
However Prisma, which is used under the hood, supports both offset and cursor pagination.
Above article summarizes pros and cons of each solution very well. My motivation comes from the fact that I am using infinite scrolling exclusively on the frontend.

Related issue: #34

This PR add cursor-based pagination that follow Prisma's syntax by:

  • adding cursor argument to GraphQL queries that returns array (also through relation)
  • adding cursor to context.query / context.db API
  • passing cursor value to Prisma

cursor argument is an object with same type as eg. connect in relation - all unique fields are allowed. Existing take and skip do not change and cursor usage is optional. Therefore this is (as fat as I can tell) non-breaking change.

Currently this PR includes some very basic unit tests. I can add more, but keep in mind that no special logic is introduced here, so extensive testing would actually test Prisma and not Keystone.

Please have a look and let me know whether you are interested in going forward with this PR. What did I miss? What other changes are needed? I would say documentation, but surprisingly I could not find anything in the docs on take and skip either ^_^

@changeset-bot

This comment was marked as resolved.

@vercel
Copy link

vercel bot commented Feb 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
keystone-next-docs ⬜️ Ignored (Inspect) Visit Preview Feb 20, 2023 at 0:00AM (UTC)

@vercel vercel bot temporarily deployed to Preview February 15, 2023 15:37 Inactive
@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 15, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ae7094c:

Sandbox Source
@keystone-6/sandbox Configuration

@dcousens
Copy link
Member

dcousens commented Feb 16, 2023

A few tests to satisfy, but the on the face of it, this pull request looks great @marekryb! 💛

surprisingly I could not find anything in the docs on take and skip either ^_^

I'm happy to not block on this pull request for that, but we should add documentation for cursor, take and skip in another PR

@vercel vercel bot temporarily deployed to Preview February 16, 2023 00:11 Inactive
@vercel vercel bot temporarily deployed to Preview February 16, 2023 00:14 Inactive
@vercel vercel bot temporarily deployed to Preview February 16, 2023 00:36 Inactive
@dcousens
Copy link
Member

@marekryb when #8320 is merged, could you please rebase?

@marekryb
Copy link
Contributor Author

@marekryb when #8320 is merged, could you please rebase?

Done. Also added few more tests.

@marekryb
Copy link
Contributor Author

@dcousens Seems everything is passing now. Looks good from my point of view.

@dcousens dcousens self-assigned this Feb 18, 2023
}`,
});
expect(errors).toEqual(undefined);
let currentOrder = 6;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should alter this to use an autoincrement identifier rather than something named order, but we won't block merging on that

@dcousens dcousens enabled auto-merge (squash) February 20, 2023 00:01
@dcousens dcousens merged commit 75add09 into keystonejs:main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants