facebook / facebook/relay

Relay spec: Cursor nullability is defined poorly, PageInfo doesn't use Cursor

Open
#3,826 3 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.