Update startCursor and endCursor in Connections.md spec

[ngbrown]

Actual usage requires that startCursor and endCursor are nullable for empty results.

This is reflected in #2122 and implemented in a17b462.

[merrywhether]

Note that similar to #2819, you should run yarn update-spec in the website package to also update the published artifacts for the spec site (after rebasing on master).

[paulofaria]

Any reason this was not merged yet? I got super confused by the spec not allowing null values.

[xenoterracide]

same, please update spec to provide an answer

[williamboman-pp]

Yes please! This confused the hell out of me.

[krvajal]

If you make the endCursor and startCursor nullable, how can you enforce at the type system level that there are not null when there are results?

[ngbrown]

@krvajal What else besides null should the cursor value be when there is no result?

A type system doesn't enforce and guarantee correctness in all situations.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[OliverCole]

Bump

[koenpunt]

If you make the endCursor and startCursor nullable, how can you enforce at the type system level that there are not null when there are results?

Maybe pageInfo should be nullable instead?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Kingdutch]

The spec is still lagging implementations here (a.k.a. "bump")

[benjie]

I approve this change 👍

Besides the general consensus in this thread, here's some (permalinks for posterity) links to Relay's codebase on main to back it up:

• Relay's types treat startCursor/endCursor as nullable:

  relay/packages/relay-runtime/handlers/connection/ConnectionInterface.js

  Lines 25 to 31 in ec03bf4

  	export type PageInfo = {
  	endCursor: ?string,
  	hasNextPage: boolean,
  	hasPreviousPage: boolean,
  	startCursor: ?string,
  	...
  	};

• Relay's "GraphQL Server Specification" uses them as nullable in the example:

  relay/website/docs/guides/graphql-server-specification.md

  Lines 69 to 74 in 8ac91d7

  	type PageInfo {
  	hasNextPage: Boolean!
  	hasPreviousPage: Boolean!
  	startCursor: String
  	endCursor: String
  	}

• There's tests on main that assert that the cursors are null:

  relay/packages/relay-runtime/handlers/connection/__tests__/ConnectionHandler-test.js

  Lines 701 to 710 in ec03bf4

  	'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo':
  	{
  	[ID_KEY]:
  	'client:4:__ConnectionQuery_friends_connection(orderby:["first name"]):pageInfo',
  	[TYPENAME_KEY]: 'PageInfo',
  	[END_CURSOR]: null,
  	[HAS_NEXT_PAGE]: false,
  	[HAS_PREV_PAGE]: false,
  	[START_CURSOR]: null,
  	},

It's clear to me that startCursor/endCursor should be nullable, are treated as nullable by Relay, and are generally implemented in the community as nullable. The specification should reflect this de facto reality.

[facebook-github-bot]

@alunyov has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[alphec]

The spec is not updated yet. I think this PR should be re-opened and most likely merged.

[lughino]

This is very frustrating. Could we reopen and merge this PR?
A wrong declaration in the official spec is causing a lot of issues.

[ngbrown]

@lughino @alphec This pull request changed the file at https://github.com/facebook/relay/blob/main/website/spec/Connections.md#introspection-2

But https://relay.dev/graphql/connections.htm has not been re-rendered from that source file. This has come up before #2457. It appears that I missed the significance of #2655 (comment). However, including the html changes in addition to the source file changes would probably have resulted in undesired merge conflicts over the 3 years it took to merge this pull request.

[lughino]

Thanks @ngbrown for raising the new PR