table icon indicating copy to clipboard operation
table copied to clipboard

Page index gets set to -1 with manual pagination and zero pages

Open lexanth opened this issue 3 years ago • 2 comments

Describe the bug

When using manual pagination, if we have zero pages of results, any attempt to set the pageIndex gives a pageIndex of -1 (which would often be invalid as an API parameter).

  • autoResetPageIndex doesn't work with manual pagination, because it resets to page zero whenever the data changes, which it inherently will do with manual pagination
  • As a workaround (or legitimate functionality, if the page count is likely to reduce while paging through), we have/had an implementation to reset the page index if it got "out of bounds". This was trying to set pageIndex to 0 or pageCount - 1 (or the greater of the two), but if pageCount = 0, react-table would "clamp" it to -1 anyway.
  • Better functionality is for the page index to reset to 0 when changing the filter/sort order, but I still think that setting pageIndex of -1 is incorrect
  • Another workaround is when passing the pageIndex parameter to the API to make sure it is "reasonable", but I don't think react-table should be setting an unreasonable value in the first place
  • Another workaround would be to say that we should never have zero pages, we should always have at least 1, even if it's empty. But that doesn't necessarily seem right either.

https://github.com/TanStack/table/blob/main/packages/table-core/src/features/Pagination.ts#L123-L139 is the relevant bit - the way that the clamp works assumes that pageCount - 1 > 0, but it isn't necessarily.

Your minimal, reproducible example

https://codesandbox.io/s/confident-roman-iwj4le?file=/src/main.tsx

Steps to reproduce

  1. Go next page
  2. Type "no_results" into the search box
  3. See that pageIndex gets set to -1, and the fake fetchData API errors because of it

Expected behavior

react-table shouldn't set pageIndex to -1 - it should not set it to < 0

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

macOS, chrome 102

react-table version

8.5.6

TypeScript version

4.5.5

Additional context

I think the workarounds are reasonable and probably give a better UX, but I still think that the clamp behaviour is wrong because it shouldn't be possible to get to pageIndex -1

Terms & Code of Conduct

  • [X] I agree to follow this project's Code of Conduct
  • [X] I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.

lexanth avatar Aug 05 '22 08:08 lexanth

We are encountering the same problem. I believe manual pagination should remove all automatic logic and only give you some helpers to check if you can go to next page or upper and lower bounds for the page index.

carlosbaraza avatar Sep 08 '22 09:09 carlosbaraza

Our problem is that we persist the pageIndex in the URL, and for some reason when navigating away and coming back to the page, while loading the data from cache and network, react-table freaks out and sets the pageIndex to -1. Our workaround was something extremely hacky and potentially bug prone (although it seems to work):

While manually handling the state updates, ensure pageIndex is never set to -1, and maintain the previous state pageIndex:

table.setOptions((prev) => ({
    ...prev,
    state,
    onStateChange: (updater) => {
      if (typeof updater === 'function') {
        const newState = updater(state);
        setState({
          ...newState,
          pagination: {
            ...newState.pagination,
            pageIndex:
              newState.pagination.pageIndex === -1
                ? state.pagination.pageIndex
                : newState.pagination.pageIndex,
          },
        });
      }
    }
})

Weird but it solves our particular niche case.

carlosbaraza avatar Sep 08 '22 09:09 carlosbaraza