Support unlimited pagination results when first is -1 or negative #2739
Conversation
Support negative `first` argument to disable pagination limits, improving query efficiency over large arbitrary limit workarounds.
spawnia
left a comment
spawnia
left a comment
There was a problem hiding this comment.
I can not even begin to consider inclusion due to the total lack of tests. Before you write any, let's discuss the issue of null vs negative values for first.
| <?php | ||
|
|
||
| declare(strict_types=1); |
There was a problem hiding this comment.
Formatting is off here and messed up in other places.
src/Pagination/PaginationArgs.php
Outdated
| ? 'simplePaginate' | ||
| : 'paginate'; | ||
|
|
||
| if ($methodName == 'paginate' && $this->first < 0) { |
There was a problem hiding this comment.
first: null seems much cleaner to me. Negative values could be the result of unchecked arithmetic errors, whereas null is much more explicit and discrete.
There was a problem hiding this comment.
For support of first: null, We need to modify in PaginationManipulator from first: Int! to first: Int. I am not sure, if that breaks other already deployed.
Sorry that I am new to php-cs-fixer and I am trying that.
There was a problem hiding this comment.
Also found this Unit Test for checking if first is Int!.
So, I am pretty much confused.
Support null `first` argument to disable pagination limits, improving query efficiency over large arbitrary limit workarounds.
2 participants
Support negative
firstargument to disable pagination limits, improving query efficiency over large arbitrary limit workarounds.Resolves #2734
Changes
Introduced a simple NegativePerPageLengthAwarePaginator