fix: Remove condition clamping pageIndex #99
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
=======================================
Coverage 99.21% 99.22%
=======================================
Files 14 14
Lines 512 515 +3
Branches 181 183 +2
=======================================
+ Hits 508 511 +3
Misses 4 4 ☔ View full report in Codecov by Sentry. |
| const pageSize = pagination.pageSize ?? DEFAULT_PAGE_SIZE; | ||
| const pagesCount = Math.ceil(items.length / pageSize); | ||
| let pageIndex = currentPageIndex ?? 1; | ||
| if (pageIndex < 1 || pageIndex > pagesCount || Number.isNaN(pageIndex)) { |
There was a problem hiding this comment.
If the subsequent pages are about to load, shouldn't the consumer set the pagesCount too? So that, if currently i'm on page 10 out of 10, and I wanna go next and I set the page number to 11, then shouldn't I also set the pagesCount to 11?
There was a problem hiding this comment.
pagesCount is calculated internally by collection hooks, not passed in
There was a problem hiding this comment.
which is the root of the problem here: when clamping the pageIndex to the calculated pagesCount, it becomes impossible to proactively set the pageIndex to the index of a lazily/not-yet loaded page, because the previous logic then reset the pageIndex to one
2 participants
Remove logic that resets current page index to 1 if a value greater than the total available number of pages is passed in. Why? This logic causes problems when trying to load subsequent pages of data from a paginated (but not filter-/sort-enabled) API.
Issue #, if available: Xw9JA4nAmlRd
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.