FleekHQ / apollo-cursor-pagination

Relay's Connection implementation for Apollo Server GraphQL library
MIT License
61 stars 12 forks source link

startCursor and endCursor should never be null #46

Open huw opened 4 years ago

huw commented 4 years ago

According to the Relay spec:

PageInfo must contain fields hasPreviousPage and hasNextPage, both of which return non‐null booleans. It must also contain fields startCursor and endCursor, both of which return non‐null opaque strings.

But in https://github.com/FleekHQ/apollo-cursor-pagination/blob/2552dfe1e4383973b6cef05272903d6da7aa972e/src/builder/index.js#L161, we don't return an appropriate fallback if the cursor is null (which happens when the query returns no results). This may (?) cause issues when integrating with Relay, but more importantly, isn't spec-compliant.

I think the appropriate fix (and I might take this on myself with a little more time) is to fall back to the cursor provided in the arguments (i.e. first or last)—these could be re-used by a client to request the theoretical 'next' page, should one become available between requests.

dmerrill6 commented 4 years ago

Hey @huw, thanks for pointing that out!

I was taking a quick stab at it, and I think I can squeeze it in a new release today. However, the specs are not very clear on how to handle this "no results" scenario. Using before and after makes sense (I guess you meant that instead of first and last which are item counts instead of cursors). But the problem with that those are optional args, so we need to decide what to return if those are not supplied. Maybe just returning "" in those cases?

Let me know!

huw commented 3 years ago

Cheers—thanks for your patience!

Yep, I meant before and after. I think you make a good point about the spec, though—there’s no guidance on what to do if there’s absolutely no data. I had a hunt around, and found this open PR on the spec—so I think we can watch that issue, and update this when there’s a solution in the spec.

dmerrill6 commented 3 years ago

Sounds good!

I will keep this open to keep track of that discussion.

Cheers!