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

More pagination options #34

Closed
JedWatson opened this issue May 6, 2018 · 4 comments
Closed

More pagination options #34

JedWatson opened this issue May 6, 2018 · 4 comments

Comments

@JedWatson
Copy link
Member

JedWatson commented May 6, 2018

We currently support two pagination options on List queries:

  • first: Int (limits the number of items returned)
  • skip: Int (skips that many items from the start, based on the sort criteria)

In addition, if we're hoping to get close to API-compatible with graph.cool / Prisma we should add support for additional pagination options in the List GraphQL APIs:

  • last: Int (reverse of first)
  • after: ID (alternative to skip, starts after the provided ID based on sort criteria)
  • before: ID (reverse of after)

I'm not sure if these are easy/possible to implement with MongoDB so it may be a Postgres-adapter specific thing.

@molomby
Copy link
Member

molomby commented Nov 28, 2018

We should definitely align with (or at least support) the Relay Cursor Connections Specification for this. Specifically...

Also, re: adapter support -- Mongo is defs capable of this but (I believe) it requires the aggregation pipeline (which I think we always use anyway?). It's easier in PG but still requires window functions to support after and before.

@molomby
Copy link
Member

molomby commented Nov 28, 2018

@molomby
Copy link
Member

molomby commented Jan 17, 2019

I know you're not a big fan, @JedWatson, but as in our other work, there probably is value in being compatible with Relay where possible. Currently there's a small but maybe important divergence here.. the Relay Cursor Spec insists that the after and before arguments be a String, not an ID. Or more specifically:

This field must return a type that serializes as a String; this may be a String, a Non‐Null wrapper around a String, a custom scalar that serializes as a String, or a Non‐Null wrapper around a custom scalar that serializes as a String.

This was an intentional choice on the part of FB; it lets you, for example, use a (stringified) timestamp as a cursor when paginating though items ordered by time.

Not sure if this makes any difference in practice for KS? Obviously IDs serialise to strings anyway, I just wonder if this might make the endpoint incompatible with some Relay tools when, otherwise, they would have worked.

Somewhat related; the spec also indicates cursor values should be opaque. Many people do this just by base64 encoding the value they want to use. This isn't a compatibility issue; just something we might want to consider.

@stale stale bot added the needs-review label Sep 21, 2019
@keystonejs keystonejs deleted a comment from stale bot Oct 9, 2019
@stale stale bot removed the needs-review label Oct 9, 2019
@stale
Copy link

stale bot commented Feb 6, 2020

It looks like there hasn't been any activity here in over 6 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contribution. :)

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

No branches or pull requests

4 participants