startCursor and endCursor should never be null

[huw]

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

apollo-cursor-pagination/src/builder/index.js

Line 161 in 2552dfe

const endCursor = edges[edges.length - 1] && edges[edges.length - 1].cursor;

, 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]

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]

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]

Sounds good!

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

Cheers!