mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.55k stars 1.33k forks source link

[data grid] combined usage of pagination and filter calls api twice with server-side data integration #15378

Open achoud444 opened 1 week ago

achoud444 commented 1 week ago

Steps to reproduce

Link to live example: (required)

Steps: 1. 2. 3.

Current behavior

I've implemented pagination and filtering, and everything works perfectly on page 1. However, when I navigate to page 2 or 3 and then apply a filter, it triggers the API call twice. Here’s a working example.

Additional questions:

  1. Is there any better way to use debounce?
  2. What is the tsx interface for apiRef?

Expected behavior

No response

Context

No response

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: [Datagrid]: Pagniation and filter calls api twice

michelengelen commented 1 week ago

@MBilalShafi another one for you ... Seems like updating with rows prop is sub-optimal here. Would be better to use the apiRef methods to update internal state ... WDYT?

@achoud444 regarding the API-interface: We do have documentation about that here.

MBilalShafi commented 1 week ago

@achoud444 When using server-side pagination and (or) filtering the rowCount should be provided to the Data Grid for it to have proper information about the number of rows on the server.

In the mentioned example, since there's no rowCount provided, the Data Grid relies on the rows prop for extracting this information.

Now, when filtering on pages > 1, we have the useEffect running twice.

  1. Due to the update of filterModel
  2. Due to the update of paginationModel (when the rows count is reduced from the previous number the Data Grid adjusts the page number automatically, so it shifts to page 1, and another trigger of useEffect happens.)

I added a rowCount in the above example to remove this from happening: https://codesandbox.io/p/sandbox/sleepy-sunset-fy2gnm?file=%2Fsrc%2FDemo.tsx

But this is far from complete, the rowCount must reflect the up-to-date row count. For example, filtering may yield less rowCount than the previous one, so the updated number should be provided to the Data Grid.

[!TIP] If you are on a Pro or Premium plan, you might give the data source a try, which is an improved way to work with server-side data.

Let me know if you have further questions. Thanks

achoud444 commented 1 week ago

@MBilalShafi

In the mentioned example, since there's no rowCount provided, the Data Grid relies on the rows prop for extracting this information.

If you seem my exaple again. I have the rowCount with the updated information from the fetchUserData. Please have a look again.

I added a rowCount in the above example to remove this from happening: https://codesandbox.io/p/sandbox/sleepy-sunset-fy2gnm?file=%2Fsrc%2FDemo.tsx

You have added twice here

If you are on a Pro or Premium plan

Yes I have a Premium plan. But still can we do something here which prevents from calling it twice?

MBilalShafi commented 1 week ago

Thanks @achoud444 for the clarification.

Yes, I overlooked that part with the rowCount already added, and adding a static rowCount suppressed the issue. However, the reason for the issue would be the same, i.e. updation of the rowCount triggers a paginationModel change due to the following line being executed which causes the unnecessary useEffect call: https://github.com/mui/mui-x/blob/ff680fdaa31db205a9bf68cee33de74e9f9a6fe5/packages/x-data-grid/src/hooks/features/pagination/useGridPaginationModel.ts#L240

I'll check and shortly update you with a possible solution.

MBilalShafi commented 1 week ago

@achoud444 I think it's the correct behavior.

Consider a scenario:

I visualized the API calls in the table below.

Call paginationModel filterModel Trigger
First { page: 1, pageSize: 5 } { items: [{ field: 'fullName', operator: 'equals', value: 'User 1' }] } User filters by "User 1"
Second { page: 0, pageSize: 5 } { items: [{ field: 'fullName', operator: 'equals', value: 'User 1' }] } Less rows than page 2, Grid shifts to page 1

Does it make sense or does your backend implementation work differently? If so, some details about its inputs/outputs would be helpful.

Or do you specifically want to skip the API call when rowCount gets equal to 0 because it doesn't make sense to fetch when there are no records?

achoud444 commented 1 week ago

@MBilalShafi

Or do you specifically want to skip the API call when rowCount gets equal to 0 because it doesn't make sense to fetch when there are no records?

In my opinion, whenever the user is on a page other than page 1 and applies a filter, the API request should be made with the filter applied and the page set to 1 ?

michelengelen commented 1 week ago

@MBilalShafi

Or do you specifically want to skip the API call when rowCount gets equal to 0 because it doesn't make sense to fetch when there are no records?

... the page set to 1 ?

That's where the second call of useEffect will run. After the filterModel change (1. run), which results in a pagination change (2. run)

MBilalShafi commented 1 week ago

In my opinion, whenever the user is on a page other than page 1 and applies a filter, the API request should be made with the filter applied and the page set to 1 ?

I think it's hard to reason about as a default behavior. What if filtering yields similar records? The page reset would be an unexpected behavior. Also, filtering doesn't necessarily yield page 1, it's basically the last page. (For example, if filtering yields 20 records, the last page would be 20/5 = 4)

The behavior could be tested with client-side filtering.

achoud444 commented 1 week ago

@MBilalShafi

What if filtering yields similar records?

Even if the records are similar, the user would still need to search for the desired row within the page. For instance, if there are 20 records with a page size of 5, and the user is on page 2 when they apply a filter that yields 15 records, they would still need to locate their result. So, wouldn’t it be better to take them directly to page 1, allowing them to start their search from the beginning?