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

[DataGrid] Log error if rowCount is used with client-side pagination #12448

Merged
merged 10 commits into from
Apr 4, 2024

Conversation

michelengelen
Copy link
Member

Added a warning that the rowCount prop has no effect when using paginationMode = client.

fixes #7303

@michelengelen michelengelen added docs Improvements or additions to the documentation component: data grid This is the name of the generic UI component, not the React module! labels Mar 14, 2024
@mui-bot
Copy link

mui-bot commented Mar 14, 2024

@cherniavskii
Copy link
Member

Should we also add a runtime warning in dev mode?
Something like:

export const propValidatorsDataGrid: PropValidator<DataGridProcessedProps>[] = [

Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

docs/data/data-grid/pagination/pagination.md Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari changed the title [docs] Added a warning concering the rowCount prop [docs] Add a warning concering the rowCount prop Mar 16, 2024
@MBilalShafi MBilalShafi changed the base branch from next to master March 21, 2024 02:19
michelengelen and others added 4 commits March 25, 2024 12:39
Co-authored-by: Bilal Shafi <bilalshafidev@gmail.com>
Signed-off-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com>
@cherniavskii cherniavskii changed the title [docs] Add a warning concering the rowCount prop [DataGrid] Log error if rowCount is used with client-side pagination Mar 26, 2024
@cherniavskii cherniavskii added dx Related to developers' experience feature: Pagination Related to the data grid Pagination feature labels Mar 26, 2024
@cherniavskii cherniavskii enabled auto-merge (squash) March 26, 2024 17:52
@michelengelen
Copy link
Member Author

@LukasTy the tests failing seem unrelated to this PR. Could you have a look since the ones failing are for DesktopDateTimeRangePicker and MobileDateTimeRangePicker?

  1) <DesktopDateTimeRangePicker /> - Describes
       Value API
         Picker open / close lifecycle
           should not call onClose or onAccept when selecting a date and `props.closeOnSelect` is false:
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/tmp/mui/packages/x-date-pickers-pro/src/DesktopDateTimeRangePicker/tests/describes.DesktopDateTimeRangePicker.test.tsx)
      at processImmediate (node:internal/timers:476:21)

  2) <MobileDateTimeRangePicker /> - Describes
       Value API
         Picker open / close lifecycle
           should not call onClose or onAccept when selecting a date and `props.closeOnSelect` is false:
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/tmp/mui/packages/x-date-pickers-pro/src/MobileDateTimeRangePicker/tests/describes.MobileDateTimeRangePicker.test.tsx)
      at processImmediate (node:internal/timers:476:21)

@LukasTy
Copy link
Member

LukasTy commented Apr 4, 2024

@michelengelen Could you first sync with master and see if it helps? 🤔
We had an actual problem with DesktopDateTimeRangePicker and MobileDateTimeRangePicker seems to be flaky. 🙈

@cherniavskii cherniavskii merged commit bb24b9e into mui:master Apr 4, 2024
15 checks passed
@michelengelen michelengelen deleted the docs/7303 branch April 4, 2024 07:35
@lauri865
Copy link
Contributor

lauri865 commented Apr 25, 2024

Is there a deeper reason for this error message or just to warn the developer @michelengelen?
We don't use server side fetching, but use filtering outside of the Datagrid. So, there's a subset of the total rows supplied to the grid in some occasions. We use rowCount to display the total number of rows in the footer.

Now we get an annoying error message. We know it doesn't have any "effect", but we need it for display reasons anyways. Unless there's an actual error, maybe warning would be more appropriate (and only in dev), otherwise, it's unnecessary code that gets bundled into production it seems?

joakimtveter pushed a commit to joakimtveter/mui-x that referenced this pull request Jun 6, 2024
mui#12448)

Co-authored-by: Bilal Shafi <bilalshafidev@gmail.com>
Co-authored-by: Andrew Cherniavskyi <andrew@mui.com>
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 dx Related to developers' experience feature: Pagination Related to the data grid Pagination feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Add a warning if paginationMode=client but rowCount > rows.length
7 participants