Skip to content
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

[data grid] rowCount with paginationMode='client' and infinite loading with onRowsScrollEnd #14735

Open
yoshegg opened this issue Sep 26, 2024 · 8 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation feature: Pagination Related to the data grid Pagination feature feature: Server integration Better integration with backends, e.g. data source support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@yoshegg
Copy link

yoshegg commented Sep 26, 2024

The problem in depth

I want to have infinite loading with DataGridPremium as described here: https://mui.com/x/react-data-grid/row-updates/#infinite-loading

I'm using TanStack Query with useInfiniteQuery which means I can trigger the loading of additional rows with a separate function (fetchNextPage), which I hand over to the onRowsScrollEnd prop. My backend provides me the total amount of data immediately on the first call. I set the rowCount prop to be this number. Unfortunately, I get an error in the DevTools console:

MUI X: Usage of the `rowCount` prop with client side pagination (`paginationMode="client"`) has no effect. 
`rowCount` is only meant to be used with `paginationMode="server"`. 

Still, the DataGrid behaves as expected: As long as I haven't loaded all the rows, it shows Total Rows: 15 of 22 in the footer. When I scroll and therefore trigger fetchNextpage, it shows Total Rows: 22. Perfect!

So is there actually a need to log this error at all?

I have prepared a MWE here: https://codesandbox.io/p/sandbox/relaxed-monad-3h5wgm

Your environment

`npx @mui/envinfo`
  System:
    OS: macOS 14.6.1
  Binaries:
    Node: 22.8.0 - ~/.nvm/versions/node/v22.8.0/bin/node
    npm: 10.8.2 - ~/.nvm/versions/node/v22.8.0/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 129.0.6668.70
    Edge: Not Found
    Safari: 17.6
  npmPackages:
    @emotion/react: 11.13.3 => 11.13.3 
    @emotion/styled: 11.13.0 => 11.13.0 
    @mui/core-downloads-tracker:  6.1.1 
    @mui/icons-material: 6.1.1 => 6.1.1 
    @mui/material: 6.1.1 => 6.1.1 
    @mui/private-theming:  6.1.1 
    @mui/styled-engine:  6.1.1 
    @mui/system:  6.1.1 
    @mui/types:  7.2.17 
    @mui/utils:  6.1.1 
    @mui/x-data-grid:  7.18.0 
    @mui/x-data-grid-premium: 7.18.0 => 7.18.0 
    @mui/x-data-grid-pro:  7.18.0 
    @mui/x-internals:  7.18.0 
    @mui/x-license:  7.18.0 
    @types/react:  18.3.9 
    react: 18.3.1 => 18.3.1 
    react-dom: 18.3.1 => 18.3.1 
    typescript:  5.6.2 

Search keywords: DataGrid paginationMode rowCount onRowsScrollEnd infinite-loading
Order ID: 95669

@yoshegg yoshegg added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels Sep 26, 2024
@github-actions github-actions bot added component: data grid This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ labels Sep 26, 2024
@MBilalShafi
Copy link
Member

@yoshegg If you have a known row count, it is recommended to use lazy-loading rather than infinite loading which gives some added benefits like prerendering the viewport so that the user can go to any page down the tree and fetch that one instead of going through pages (or chunks) one by one.

I tried to update your example with lazy loading and it doesn't show the error anymore: https://codesandbox.io/p/sandbox/keen-lucy-vvlnh3

Let me know if it makes sense.

Sidenote: I moved some static variables like columns and rows outside the component scope as it's a recommended practice to stabilize the reference for performance reasons. Check this frequently asked question for details.

@MBilalShafi MBilalShafi added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 27, 2024
@yoshegg
Copy link
Author

yoshegg commented Oct 1, 2024

@MBilalShafi thank you. We want to use infinite loading because it works easily with TanStack Query's useInfiniteQuery and our existing backend that uses pagination. So my question remains:

So is there actually a need to log this error at all?

Thanks for pointing out the change to columns and rows. For the sake of the MWE, I did not bother to memoize.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Oct 1, 2024
@michelengelen michelengelen changed the title [question] rowCount with paginationMode=client and infinite loading with onRowsScrollEnd [data grid] rowCount with paginationMode='client' and infinite loading with onRowsScrollEnd Oct 1, 2024
@michelengelen michelengelen added feature: Pagination Related to the data grid Pagination feature feature: Server integration Better integration with backends, e.g. data source labels Oct 1, 2024
@MBilalShafi
Copy link
Member

MBilalShafi commented Oct 3, 2024

So is there actually a need to log this error at all?

By logging the error, the intention is to be clear that infinite loading is supposed to be a client-side feature. That means that even if the data is being loaded from a server, the user is handling it and reacting to changes like filtering and sorting outside the data grid scope. (Basically resetting the rows prop on every update)

However, that changes with the new lazy-loading feature with the data source which combines both lazy-loading and infinite loading both of which essentially corresponds to the presence or absence of the rowCount prop.

Coming to your question, I'd say to ignore the error for now if the use-case works for you (it's not shown in production anyway) and aim to switch to the data source variant of infinite loading since the current infinite loading feature is most probably going to be deprecated and removed ultimately.

CC @arminmeh Feel free to add something if you want.


By the way, as far as I understand, the useInfiniteQuery shouldn't require you to pass the rowCount to the Data Grid, since it provides props like hasNextPage and fetchNextPage to smoothline data fetching. Is there a specific reason you want to pass the rowCount to the Grid?
You might consider customizing the Footer component if it's just for showing the total row count in the footer.

@MBilalShafi MBilalShafi added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 3, 2024
@arminmeh
Copy link
Contributor

arminmeh commented Oct 4, 2024

The new feature will also not allow showing total number of rows in combination with infinite loading. It will automatically fill the grid with skeleton rows to match the count.

Regarding the error itself, I see that we have an inconsistency there.
In the docs it is stated that this does not work with the client side pagination, but in the component we pass this value regardless.
So, we should either update the docs and remove the error or add a check to prevent this being used client side.

@cherniavskii
Copy link
Member

I did a quick search and it looks like rowCount might be useful in some cases with client-side pagination: #12448 (comment)
Is it a legit use case?

@yoshegg
Copy link
Author

yoshegg commented Oct 8, 2024

@MBilalShafi

Coming to your question, I'd say to ignore the error for now if the use-case works for you (it's not shown in production anyway) and aim to switch to the data source variant of infinite loading since the current infinite loading feature is most probably going to be deprecated and removed ultimately.

That's what we are doing right now. But we remove it? It works perfectly fine.

By the way, as far as I understand, the useInfiniteQuery shouldn't require you to pass the rowCount to the Data Grid, since it provides props like hasNextPage and fetchNextPage to smoothline data fetching. Is there a specific reason you want to pass the rowCount to the Grid?

Yes, and that's what I'm using. But: If I don't provide the rowCount prop, the footer only contains the number of loaded rows that will then increase every time I scroll to the bottom of the table. It's nicer for the user to also see the total amount of entries.

You might consider customizing the Footer component if it's just for showing the total row count in the footer.

In this case I would just recreate the exact footer that is shown right now just to get rid of the error being logged.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Oct 8, 2024
@michelengelen
Copy link
Member

I did open a new issue to change the log type from error to a warning: #14887

@MBilalShafi
Copy link
Member

MBilalShafi commented Oct 9, 2024

Is it a legit use case?

Yes, it seems like something that should reduce some friction, I'd also be in favor of making it easier to customize the rowCount value in the Footer since customizing the Footer slot is not a very straightforward task. Mentioning in the documentation, converting from error to warning, and updating the text accordingly should help.

@MBilalShafi MBilalShafi added docs Improvements or additions to the documentation and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation feature: Pagination Related to the data grid Pagination feature feature: Server integration Better integration with backends, e.g. data source support: commercial Support request from paid users support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

No branches or pull requests

5 participants