Material-ui: TablePagination warning when page is out of range

Created on 7 May 2019  路  23Comments  路  Source: mui-org/material-ui

This change adds a warning in a common use case.

Let's say a user is displaying a list with two pages. The user goes to page 2. Then, he chooses to refresh the page. The console shows a warning:

Warning: Failed prop type: Material-UI: the page prop of a TablePagination is out of range (0 to 0, but page is 1).

This is because, when the user refreshes the page, the list is empty (count=0) until the data is loaded. This "loading" state fails the page prop validation, which creates a warning.

This used to work well in v1, I'm not sure this regression is a desired effect.

_Originally posted by @fzaninotto in https://github.com/mui-org/material-ui/pull/14534#issuecomment-490079685_

Table discussion

Most helpful comment

@fzaninotto In your example, have you considered?

start from 0

const PaginationWithLoading = () => {
  const [count, setCount] = useState(-1);
  // simulate loading
  useEffect(() => {
    setTimeout(() => setCount(15), 1000);
  });
  return (
    <TablePagination
      page={count === -1 ? 0 : 1}
      count={count === -1 ? 0 : count}
      rowsPerPage={10}
      onChangePage={() => null}
    />
  );
};

or

start from page 1

const PaginationWithLoading = () => {
  const [count, setCount] = useState(-1);
  // simulate loading
  useEffect(() => {
    setTimeout(() => setCount(15), 1000);
  });
  return (
    <TablePagination
      page={1}
      count={count === -1 ? 1 * 10 + 1 : count}
      rowsPerPage={10}
      onChangePage={() => null}
    />
  );
};

All 23 comments

This is a question where prop validation should reside. In this case I think app code is more appropriate. The library can't know if the current value will make sense later. Checking for 0,0 is probably a good enough heuristic but it's yet again another heuristic the user has to know.

I think it's better that the app dev decides what fallback value should be used (min, max etc) until the data is loaded rather than the library. I'm inclined to believe that the warning is fine here. It's exactly the reason it is only a warning: Your code might not work but we can't be sure so we warn instead of throwing.

@fzaninotto Could you include a codesandbox that illustrates the use case?

Here is the CodeSandbox: https://codesandbox.io/s/zkqpo139rx

Here is the CodeSandbox: codesandbox.io/s/zkqpo139rx

That doesn't really help me understand the issue. The decision to warn against out-of-range pages was a conscious one. What I'm missing is are use cases where this is accepted behavior and why the warning is appropriate in those cases.

I've modified the code. Is it more explicit?

I agree with @eps1lon, I think the the warning is correct here. It's the least worse solution to the problem. @fzaninotto Do you have a better alternative in mind?

Not warning at all if the page is out of bounds when the count is zero seems a reasonable trade off to me.

@fzaninotto In your example, have you considered?

start from 0

const PaginationWithLoading = () => {
  const [count, setCount] = useState(-1);
  // simulate loading
  useEffect(() => {
    setTimeout(() => setCount(15), 1000);
  });
  return (
    <TablePagination
      page={count === -1 ? 0 : 1}
      count={count === -1 ? 0 : count}
      rowsPerPage={10}
      onChangePage={() => null}
    />
  );
};

or

start from page 1

const PaginationWithLoading = () => {
  const [count, setCount] = useState(-1);
  // simulate loading
  useEffect(() => {
    setTimeout(() => setCount(15), 1000);
  });
  return (
    <TablePagination
      page={1}
      count={count === -1 ? 1 * 10 + 1 : count}
      rowsPerPage={10}
      onChangePage={() => null}
    />
  );
};

Not sure I understand. You suggest to pass a fake but valid count to TablePagination with the data is loading?

@fzaninotto I'm suggesting to fully assume the table state, either we don't know anything and we display 0 or we optimistically assume the page contains items.

Interesting. I made the choice to hide the TablePagination during loading, but that creates a flickering effect when the loading ends and the pagination appears. Maybe showing an optimistic pagination would be less perturbing. I'll test that on a few users.

It boils down to designing the loading state. That's something the Material Design specification doesn't address if I'm not mistaken, and it's always a hard task.

To reiterate on another point that was addressed by this change: Passing an out-of-range page could cause bugs depending on how onPageChange was used. onPageChange used to be called on every render if the page was out-of-range.

@fzaninotto Have you found an acceptable solution to the problem?

Yes, I've hidden the pagination while loading. It adds a flicker, but there is one anyway (since the total number of results changes).

Thanks for your help.

Yes, I've hidden the pagination while loading.

Why not just check against an out-of-range page or use page={0} for the loading state?

Why not just check against an out-of-range page or use page={0} for the loading state?

Displaying an out of range page or a wrong page number during loading is worse than displaying nothing IMO, because it's showing information that we know are wrong.

is it Fixed in v4.9.3 ????
I have some trouble dealing with the issues...

still there is error at the latest version.. fix properly please

If you do this:
useEffect(() => { if( data.length === rowsPerPage && page > 0 ) { setPage(0); } }, [data.length, rowsPerPage, page]);

And this:
<TableFooter> <TableRow> <TablePagination rowsPerPageOptions={[5, 10, 25, { label: 'All', value: -1 }]} colSpan={3} count={data.length} rowsPerPage={rowsPerPage} page={ ( page > 0 && data.length === rowsPerPage ) ? 0 : page } SelectProps={{ inputProps: { 'aria-label': 'rows per page' }, native: true, }} onChangePage={handleChangePage} onChangeRowsPerPage={handleChangeRowsPerPage} ActionsComponent={TablePaginationActions} /> </TableRow> </TableFooter>

Then it fixes the issue.

Set your page property on your like this:
page={ ( page > 0 && data.length === rowsPerPage ) ? 0 : page }

Have you all considered if someone has a largeish data set and wants to be able to start in the middle, how to handle it? For example, if I have 1000 records and i do the page length of 50 per page, and i want to start at page 10 (without having to pre-load 500 records), how could i accomplish this?

I was trying to do so with using queryParams (pageNumber), and then loading the page via the api, however if I use pageNumber in with the TablePagination, I get:
index.js:1 Warning: Failed prop type: Material-UI: the page prop of a TablePagination is out of range (0 to 0, but page is 1).

@lostmarinero Do you have a small reproduction we could look at? The TablePagniation component is only concerned with the display, you could "lies" to him around what data is available.

So still no stable solution found?

Maybe related: https://github.com/mbrn/material-table/issues/955 (for me, the data changes when I'm on page 2, and the table doesn't update the page when data is changed, so the page is now out of bounds).

Yes, I've hidden the pagination while loading. It adds a flicker, but there is one anyway (since the total number of results changes).

Thanks for your help.

Can you show how you hid the pagination on load?

Was this page helpful?
0 / 5 - 0 ratings