Pagination.data should be Non-nullable with Non-nullable Types

[99linesofcode]

I've seen a couple of issues and pull requests (#872, #1012) about this with - in my opinion - rather complicated/involved changes. I've been looking at this earlier today and, based on the documentation here, I have come to the following conclusion:

The data field on the PaginationType should be a non nullable array (as it is always an array). Similarly the elements inside that array should all be Non-nullable Types since either they do not exist OR they are of the GraphQL type you are paginating.

Posting this as a discussion as I want to make sure that my reasoning is sound. Happy to take a stab at this!

[mfn]

Thanks for bringing this up!

Reminds me I totally forgot about #1012 but when I checked it out again, I still have no clue why the changes are the way they are. I just haven't gotten time to think more about this, being more focused on getting the new version out and making sure it's solid.

Coming back to the topic:

The data field on the PaginationType should be a non nullable array

…

the elements inside that array should all be Non-nullable Types
…

I have not looked at the impact on changing this, but on principle, I absolute agree with this! It's very unlikely someone needs nullables in their paginated types (if you ask me, it feels like a mistake that types are "nullable" by default), if they need it, they should roll their own.

[99linesofcode]

Don't mention it 😄

I don't really see any breaking change here. If people assumed nullable somewhere then that just means they have some defensive programming in place that is no longer necessary. Looking at the source code for the PaginationType as well as Laravel's AbstractPaginator a collection is returned which is never nullable. For as long as the getCollection() method has existed.

[99linesofcode]

Closing this so that we can continue the discussion in #1012.