Relay spec: Cursor nullability is defined poorly, PageInfo doesn't use Cursor
- Dominant language
- Rust
- Stars
- 19k
- Forks
- 1.9k
- PR merge metrics
- No merged PRs in 30d
Description
The current spec [defines a `Cursor`](https://github.com/facebook/relay/blob/f757b393f372e97b4655305c2b3ea0c4f26c2623/website/spec/Connections.md#cursor "Cursor definition") like so:
> An "Edge Type" must contain a field called `cursor`. 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.
>
> Whatever type this field returns will be referred to as the _cursor type_ in the rest of this spec.
This itself is a little weird. Why can an edge type have a nullable cursor? The only reasonable meaning for a null cursor is to represent a position that does not correspond to any edge or lie in between edges. The most obvious usage here is when returning a `PageInfo` when there are no edges at all, the start/end cursors would then be `null`.
To that end, `cursor` should be defined as strictly Non-Null, and then the places that use cursors would then specify either a nullable or a non-null cursor. The Edge Type field `cursor` would itself specify that it is a non-null cursor.
On that note, the current spec [defines `PageInfo`'s fields](https://github.com/facebook/relay/blob/f757b393f372e97b4655305c2b3ea0c4f26c2623/website/spec/Connections.md#fields-2 "PageInfo field definition") like so:
> `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 opaque strings. The fields `startCursor` and `endCursor` can be null if there are no results.
This type has two cursors, and yet it's not even using the _cursor type_ as defined earlier. The introspection for this type also explicitly uses nullable strings without any note that other types are possible (as compared with the introspection for Cursor which has a comment indicating that it represents a cursor type of `String!` but other types are possible).
This is a problem because it means in any strongly-typed language, the `startCursor` and `endCursor` fields are incompatible with the forward/backward pagination arguments and with the `cursor` field on an edge.
`PageInfo` should instead be defined such that `startCursor` and `endCursor` are nullable _cursor types_, where null is used if there is no data. This is similar to today but it just means changing "opaque string" into "_cursor type_" and putting a comment in the introspection indicating that this example shows the cursor type as being a `String`.
Contributor guide
Research direction
Read website/spec/Connections.md, especially the Cursor and PageInfo field definitions and their introspection examples. Update the spec so edge cursors are non-null and PageInfo startCursor/endCursor use nullable cursor types, with the examples and comments reflecting the cursor type.】【。
Written by the indexing model from the issue text.
Assessment
- Tech stack
- graphql
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100