Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PageInfo type field nullability different from specification #243

Open
Cito opened this issue May 3, 2020 · 7 comments
Open

PageInfo type field nullability different from specification #243

Cito opened this issue May 3, 2020 · 7 comments

Comments

@Cito
Copy link
Member

Cito commented May 3, 2020

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
Copy link
Member Author

Cito commented May 3, 2020

This should also solve #240.

@olso
Copy link

olso commented Jul 16, 2020

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

@numberten
Copy link

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.

@GabrielGil
Copy link

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?

@Cito
Copy link
Member Author

Cito commented Mar 19, 2021

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

@bamorim
Copy link

bamorim commented Mar 29, 2021

@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.

@IvanGoncharov
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants