[BUG] `useTable` from `@refinedev/react-table` causes infinite rendering

khoaxuantu commented 2 months ago

Describe the bug

I was trying to create a list view following up the example of useTable. However, when I got to the page, the page was rendering infinitely as the following:

github-issue-reproduction

It seems like the tableQuery's status remains its fetchStatus as "fetching", but there is no calling to the data provider's getList client action, and the data remains undefined, so no data can be displayed.

Steps To Reproduce

My code is mostly the same with the usage example. The difference is it is built on top of Next.js's app routing.

Here is my Refine's layout:

<Refine
  routerProvider={routerProvider}
  dataProvider={DataProviderClient}
  authProvider={authProvider}
  notificationProvider={useNotificationProvider}
  resources={[
    {
       name: "users",
       list: "users",
       create: "users/create",
       edit: "users/:id/edit",
       show: "users/:id",
    },
  ]}
  options={{
    syncWithLocation: true,
    warnWhenUnsavedChanges: true,
    useNewQueryKeys: true,
  }}
>
  {children}
</Refine>

Here is my page at /src/app/users/page.tsx

"use client";

import { HStack, Table, TableContainer, Tbody, Td, Th, Thead, Tr } from "@chakra-ui/react";
import { ColumnFilter } from "@components/DataDisplay/ColumnFilter";
import { Pagination } from "@components/Pagination";
import { capitalize } from "@lib/helpers/string.helper";
import { DateField, EditButton, List, ShowButton, TagField, TextField } from "@refinedev/chakra-ui";
import { useTable } from "@refinedev/react-table";
import { ColumnDef, flexRender } from "@tanstack/react-table";
import { useMemo } from "react";
import { UserProps } from "./schema/user.schema";

const LIST_PROPS_AS_TEXT = ["name", "email"];

export default function UsersPage() {
  const columns = useMemo<ColumnDef<UserProps>[]>(
    () => [
      {
        id: "_id",
        header: "ID",
        accessorKey: "_id",
        meta: {
          filterOperator: "eq",
        },
      },
      ...LIST_PROPS_AS_TEXT.map((field) => ({
        id: field,
        header: capitalize(field),
        accessorKey: field,
      })),
      {
        id: "roles",
        header: "Roles",
        accessorKey: "roles",
        cell: (props) => {
          return (
            <HStack>
              {(props.getValue() as string[]).map((role: string, index) => (
                <TagField key={index} value={role} />
              ))}
            </HStack>
          );
        },
      },
      {
        id: "createdAt",
        header: "Created At",
        accessorKey: "createdAt",
        cell: (props) => <DateField value={props.getValue() as string} format="HH:mm DD/MM/YYYY" />,
      },
      {
        id: "actions",
        header: "Actions",
        accessorKey: "_id",
        cell: (props) => {
          return (
            <HStack>
              <EditButton recordItemId={props.getValue() as string} />
              <ShowButton recordItemId={props.getValue() as string} />
            </HStack>
          );
        },
      },
    ],
    [],
  );

  const {
    getHeaderGroups,
    getRowModel,
    refineCore: { tableQuery, pageCount, current, setCurrent },
  } = useTable<UserProps>({ columns });

  console.log("🚀 ~ UsersPage ~ Render", tableQuery);
  const total = tableQuery?.data?.total ?? 0;

  return (
    <List>
      <p>Total: {total}</p>
      <TableContainer marginTop={12}>
        <Table variant="simple">
          <Thead>
            {getHeaderGroups().map((headerGroup) => {
              return (
                <Tr key={headerGroup.id}>
                  {headerGroup.headers.map((header) => {
                    return (
                      <Th key={header.id}>
                        {!header.isPlaceholder && (
                          <HStack spacing={2}>
                            <TextField
                              value={flexRender(
                                header.column.columnDef.header,
                                header.getContext(),
                              )}
                            />
                            <ColumnFilter<UserProps> column={header.column} />
                          </HStack>
                        )}
                      </Th>
                    );
                  })}
                </Tr>
              );
            })}
          </Thead>
          <Tbody>
            {getRowModel().rows.map((row) => {
              return (
                <Tr key={row.id}>
                  {row.getVisibleCells().map((cell) => {
                    return (
                      <Td key={cell.id}>
                        {flexRender(cell.column.columnDef.cell, cell.getContext())}
                      </Td>
                    );
                  })}
                </Tr>
              );
            })}
          </Tbody>
        </Table>
      </TableContainer>
      <Pagination current={current} pageCount={pageCount} setCurrent={setCurrent} />
    </List>
  );
}

Expected behavior

I expect to have the data rendered when visiting the page.

Packages

├── next@14.2.5 ├── react@18.3.1 ├── react-dom@18.3.1 ├── @refinedev/react-table@5.6.13 ├── @tanstack/react-table@8.20.1 ├── @refinedev/core@4.54.0 ├── @refinedev/nextjs-router@6.1.0 ├── @refinedev/chakra-ui@2.32.0 ├── @chakra-ui/react@2.8.2

Additional Context

Btw, I saw in the source code that it may pass an empty array [] to the TanStack's reactTable here.

Wondering if it has any effect on the infinite rendering of useTable. Because there was a TanStack's issue that is quite similar to it.

BatuhanW commented 2 months ago

Hey @khoaxuantu, I couldn't reproduce the issue from the code you've provided. Could you provide a repository with a minimal repro? That would be great! Thanks.

khoaxuantu commented 2 months ago

@BatuhanW Sure! It's quite complicated to reproduce minimally, but I just have created a branch of my project here to reproduce it. I have mocked the login and the data so that we can visit the /users page and look into the issue directly. Would you mind checking it out? Thanks

aliemir commented 2 months ago

Just checked out the codebase you've provided @khoaxuantu. I was able to reproduce the same rendering issue with the initial setup. Then it resolved when I removed the onClick prop of the prev button in the <Pagination /> component. Can you check if this works? Instead of using current - 1, I've updated it to pagination.prev, not sure if this is something related with the caches but it started working on every test I make 😅

khoaxuantu commented 2 months ago

@aliemir It doesn't work 100% correct, sadly. I have tried both current - 1 and pagination.nev in the <Pagination /> component as you said. The issue in both occurred as follows:

From the root path /, I navigated to the /users (via navigation sidebar), I got the issue.
Then I clicked on the pagination buttons, and it works. It did call the getList action, fetch the data successfully, and stop rendering infinitely.
Then I navigated to /, then reloaded the whole site, and clicked on the Users navigation sidebar again, the issue occurred again

Another thing that I noticed is that if I visit the /users path by entering the url directly, the page is renderred successfully. So I don't think the problem is from the pagination component

aliemir commented 2 months ago

Hey @khoaxuantu you are right! My bad, I only tested out in the same page and hot reloading fixed the issue when I made changes in the <Pagination /> 🤦 I investigated a bit more into the issue, it looks like its only related to the useTable of @refinedev/react-table, when I switch to using useTable from @refinedev/core the issue did not occur.

I've found that the issue happens when syncing sorters and filters from React Table to Refine. We did not had a check if the current states are equal or not and left with repeated calls to setFilters. This caused queries to stuck at loading without properly calling the API. As a workaround I saw that setting syncWithLocation to false resolves the issue. For a fix, we need to add equality checks for filters and sorters in their effects.

When the issue is stuck at loading, Refine's overtime interval keeps running, this was causing your console.log to run repeatedly every second. It looks like an infinite rendering issue but its really logging due to overtime.elapsedTime getting updated 😅

In my local, I've tested out using lodash/isEqual before calling setFilters in a useEffect of useTable and it seems to resolve the issue 🤔

Let us know if syncWithLocation: false workaround is working for you, then we can discuss about the fix. Also let us know if you want to work on the fix, we'll be happy to see your contribution 🙏

khoaxuantu commented 2 months ago

Hey @aliemir, I just tried setting syncWithLocation to false and it worked as expected. I think I will go with this workaround for now. Because of my limited time, I think it's better to let you fix the issue and I hope to be able to turn on the option in the future releases soon. Thank you very much for the support :bow:

Anonymous961 commented 1 month ago

Hey @aliemir, were you suggesting something like this?

if (!isEqual(crudFilters, filtersCore)) {
      setFilters(crudFilters);
}

aliemir commented 1 month ago

Hey @Anonymous961 yeah this is what I tried, would love to hear from you if you can spare some time to work on this 🙏

chankruze commented 1 month ago

Hey @aliemir, were you suggesting something like this?
if (!isEqual(crudFilters, filtersCore)) {
      setFilters(crudFilters);
}

Ye something like that!

qianzhong516 commented 1 month ago

Hey hey, I've been using refine recently and really fell in love with this amazing product. Can I work on this issue?

qianzhong516 commented 1 month ago

Hey hey, I've been using refine recently and really fell in love with this amazing product. Can I work on this issue?

This is NOT hacktoberfest, if you can fix, fix and submit PR. Open source is ruined literally....

Hey, first of all, if this repo isn't up for hacktoberfest, it's fine. I want to fix this issue genuinely out of my likeness towards the product. Secondly, please do not make this assumption about anyone that participates in the hacktoberfest event. It's hurtful to see your comment as someone who is literally interested in contributing to the product. Your message is very unwelcoming and opinionated to me.

I asked to be assigned with the task just to make sure that no one else has been working on the issue because it seems to have a long discussion going on.

qianzhong516 commented 1 month ago

@qianzhong516 Anyways It was my opinion indeed. And I didn't know you are a female. I don't know since what time you are contributing to OSS, but I have never seen someone getting assigned to a project to fix an issue until unless it's an enterprise application. Generally it's the internal team/staff gets assigned to priority issues, rest are open for everyone for first come first serve.

In the other hand, what you have to loose ? Why don't you fix it and submit a PR even though someone is working. It is not fixed that there is a particular solution to an issue. If more then one PR is raised, the quality/optimized solution will be merged. So, it doesn't matter if someone is working or not, from your profile I learnt that you are passionate, want to be a full stack dev, I don't think you need to be assigned to do things. You are born to fix issues and raise PR asap.

And I didn't know you are a female. I don't know since what time you are contributing to OSS,

Does any of these facts validate that you can be unwelcoming to someone who wants to be a part of an open source project? Like because the person is a female, because you don't know when they started contributing? Is it that hard for you just to say "yep, feel free to go ahead"?

Are you sure your actions here adhere to the policies in this repo? I literally have never seen someone being this agressive for no reason. If this gets allowed, I would be pretty surprised.

I thought you were just having a bad day but now it just seems you are full of disrespect for other people. Please stop right now.

qianzhong516 commented 1 month ago

@qianzhong516 reasons for NOT being welcoming:

Haven't you seen up for grabs tag for this issue ?

Haven't you seen open status ?

Haven't you seen there is NO PR regarding this issue ?

Only if you have checked these things, instead of reading policy, you would have welcomed.

Secondly, I believe your understanding is not that sharp cause you have miss understood my previous comment. Anyone with a good understanding in english would know that was not disrespecting at all. Please read that again (with translator if needed).

Thirdly, I am no one to welcome you to a project, so why my comments are bothering you ? If you are truly here for your passion, you could have proved that with a PR instead of misinterpretation, instead you are trying to gain sympathy (women ☕)

Reported.

BatuhanW commented 4 weeks ago

@qianzhong516 reasons for NOT being welcoming:
1. Haven't you seen `up for grabs` tag for this issue ?

2. Haven't you seen `open` status ?

3. Haven't you seen there is NO PR regarding this issue ?
Only if you have checked these things, instead of reading policy, you would have welcomed.

Secondly, I believe your understanding is not that sharp cause you have miss understood my previous comment. Anyone with a good understanding in english would know that was not disrespecting at all. Please read that again (with translator if needed).

Thirdly, I am no one to welcome you to a project, so why my comments are bothering you ? If you are truly here for your passion, you could have proved that with a PR instead of misinterpretation, instead you are trying to gain sympathy (women ☕)

@chankruze

Haven't you seen our Code of Conduct?
Haven't you seen Our Standards section?

Only if you have checked these things, you would understand what you are doing is unacceptable.

raulmar0 commented 1 week ago

Can this bug be assigned to me?

Anonymous961 commented 1 week ago

@raulmar0 feel free to start working on this issue and submit a pull request when you're ready. No need to wait for it to be assigned to you.

refinedev / refine