Skip to content

fix: unnecessary setPageIndex to the last page when pageIndex is 0 #471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

StinsonZhao
Copy link
Contributor

@StinsonZhao StinsonZhao commented Jan 9, 2025

//if page index is out of bounds, set it to the last page
useEffect(() => {
  if (!enablePagination || isLoading || showSkeletons) return;
  const { pageIndex, pageSize } = pagination;
  const firstVisibleRowIndex = pageIndex * pageSize;
  if (firstVisibleRowIndex >= totalRowCount) {
    table.setPageIndex(Math.ceil(totalRowCount / pageSize) - 1);
  }
}, [totalRowCount]);

The logic within this useEffect is designed to detect when the pageIndex exceeds its valid range. If such an occurrence is detected, it resets to the last page. The second ifstatement within this logic, however, poses certain issues. If the current page is already the first one (with pageIndex being 0) and the totalRowCount is 0 (indicating an empty table), this if statement would validate, but in fact, there is no need to invoke setPageIndex to reset to the last page because the current pageIndex is already the final page. This unnecessary invocation of setPageIndex not only creates redundant function calls but also leads to other issues.

We utilize TanStack Query to request data: const query = useQuery(...), and we then pass query.isLoading into MantineReactTable. Due to the caching mechanism, isLoading only remains true during the initial query request; it is false afterwards. Should the total returned by this query remain 0, it will trigger the aforementioned if logic, causing any component subsequent to the query request to initiate an onPaginationChange (triggered by the setPageIndex method) upon mounting.

Copy link

vercel bot commented Jan 9, 2025

@StinsonZhao is attempting to deploy a commit to the Kevin Vandy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@alessandrojcm alessandrojcm merged commit 33cda81 into KevinVandy:v2 Jan 27, 2025
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants