graphql-relay-js icon indicating copy to clipboard operation
graphql-relay-js copied to clipboard

PageInfo type field nullability different from specification

Open Cito opened this issue 4 years ago • 7 comments

The relay spec says that "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."

However, the Flow type for PageInfo makes all of them nullable, and the GraphQL object type for PageInfo makes startCursor and endCursor nullable.

Can we fix these types to conform to the specification?

Cito avatar May 03 '20 17:05 Cito

This should also solve #240.

Cito avatar May 03 '20 17:05 Cito

Just ran into this

Screenshot 2020-07-16 at 17 58 14

I'm doing this until its fixed

Screenshot 2020-07-16 at 18 04 38

olso avatar Jul 16 '20 15:07 olso

Even though the spec claims startCursor and endCursor must be non-null, I believe they need to support null to handle where the edges return an empty list.

numberten avatar Aug 18 '20 18:08 numberten

Indeed. I have run into the same incongruence, and it is not clear what option is correct. Either the spec is just ambiguous or out of date, or maybe this library is just taking a different approach.

Either way, in case the spec is respected, and both startCursor and endCursor are not null opaque strings, what should be their value when there are no results? Should the connection be null? Should the cursors be an empty string?

GabrielGil avatar Sep 14 '20 15:09 GabrielGil

It seems this has been (accidentally?) solved by #300 already.

Cito avatar Mar 19 '21 23:03 Cito

@Cito thanks for reporting that, just stumbled upon this as I was confused about the spec and some implementations out in the wild.

I seems that #300 still marks the endCursor and startCursor as nullables:

  startCursor: ConnectionCursor | null,
  endCursor: ConnectionCursor | null,

https://github.com/graphql/graphql-relay-js/pull/300/files#diff-c906c9bb7d4a80f721e5ef3e8c7159dda11e5f8829ace2f04982a460a453c0a2R12-R13

It only fixes the nullability of hasPreviousPage and hasNextPage.

bamorim avatar Mar 29 '21 17:03 bamorim

@Cito Thanks for reporting it spec should definitely be clarified about the values of startCursor and endCursor in case of an empty array.

IvanGoncharov avatar May 31 '21 10:05 IvanGoncharov