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

[lilyball]

The current spec defines a Cursor 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 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.

[lilyball]

The Edge Type field cursor would itself specify that it is a non-null cursor.

To clarify this a bit, having a nullable cursor on an edge (as allowed by the current spec) conflicts with the rest of the spec. In particular, if an edge could have a null cursor then the following issues would happen:

• When passing pagination arguments, specifying after: null or before: null would be required to behave differently than omitting the field arguments entirely. A close reading of the GraphQL spec does indicate that passing null vs omitting is exposed to field execution¹, but there is a practical difficulty here in that relying on this difference makes it harder to write reusable queries as you cannot conditionally include an argument in a query (arguments do not allow for directives).

  Also note that ApplyCursorsToEdges (spec) is defined using language like "If after is set:". This technically does mean that this condition should trigger on after: null, but that requires searching for a null cursor in all edges even if no edge actually has a null cursor. This is also confusing when the cursor is explicitly defined as something like String! as null cannot match any edge. I fully expect that most implementations treat this as "if after has a value other than null", but this definition prevents fetching the page before or after an edge with a null cursor. Requiring the cursor type to be non-null and changing the pagination algorithm to specify non-null rather than "set" simplifies the model and removes ambiguity.

• PageInfo is defined (spec) such that

  startCursor and endCursor must be the cursors corresponding to the first and last nodes in edges, respectively.

  The current spec does say these "can be null if there are no results", although it really should say "must be" as passing anything else is incompatible with the requirement that they correspond to the first and last nodes in edges. However if an edge had a null cursor, then the startCursor or endCursor could be null despite the page having edges. Clients can disambiguate here by checking if the page has edges before determining how to interpret a null start/end cursor, but this is confusing and will most likely be overlooked by most clients writing queries against such an API.

If any implementation has a valid reason for wanting to support an edge with a null cursor, that could be handled by defining the cursor type as a custom scalar and serializing the null cursor as a sentinel scalar value instead (such as an empty string). This means that there is no reason why cursors cannot be required to be nonnull.

Footnotes

1. §6.4.1 says that argumentName: null would add this entry to coercedValues whereas omitting it would not add the argument to coercedValues unless a default value is defined. ↩

[nrbnlulu]

graphql/graphql-relay-js#243 I don't use relay just saw these two issues and though they are related, sorry if I'm wrong.

[jedwards1211]

@lilyball

If any implementation has a valid reason for wanting to support an edge with a null cursor

I think you're a bit too worried about this, it's pretty obvious that the intent is to use a unique string value for every position in the data. It's not necessary to require that edge.cursor be non-null, only that after: null must have the same behavior as not passing after at all, and likewise for before.

I'm sure edge.cursor is allowed to be null to allow APIs to skip computing the cursor of every single edge in the results, since you only need startCursor and endCursor of each non-empty page to paginate backward and forward, and computing the cursors in between could be a waste of effort. It actually doesn't seem necessary to specify an edge.cursor field in the first place. But for a use case where you need to reference the position of each node for some reason, it's convenient for that field name to be standardized, I guess...