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

[Bug]: DataGrid controlled select is slow with 50+ records #33402

Open
2 tasks done
rocketBANG opened this issue Dec 4, 2024 · 3 comments
Open
2 tasks done

[Bug]: DataGrid controlled select is slow with 50+ records #33402

rocketBANG opened this issue Dec 4, 2024 · 3 comments

Comments

@rocketBANG
Copy link

Component

DataGrid

Package version

9.56.1

React version

18.3.0

Environment

System:
    OS: Windows 11 10.0.22631
    CPU: (16) x64 AMD EPYC 7763 64-Core Processor
    Memory: 23.49 GB / 63.95 GB
  Browsers:
    Edge: Chromium (131.0.2903.63)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    @fluentui/react-calendar-compat: ^0.1.21 => 0.1.21
    @fluentui/react-components: ^9.56.1 => 9.56.1
    @fluentui/react-datepicker-compat: ^0.4.54 => 0.4.54
    @fluentui/react-icons: ^2.0.265 => 2.0.265
    @fluentui/react-nav-preview: ^0.10.0 => 0.10.0
    @fluentui/react-timepicker-compat: ^0.2.38 => 0.2.38
    @types/react: ^18.3.2 => 18.3.2
    @types/react-dom: ^18.3.0 => 18.3.0
    react: ^18.3.1 => 18.3.1
    react-dom: ^18.3.1 => 18.3.1

Current Behavior

Following the default example for a controlled multiselect data grid, there are performance issues when the row count is 50+ (in the example it is 250, but still noticeable at 50 rows. We use the virtualized version in our project and still see the issue as there is still a lot of rows on the screen)

https://stackblitz.com/edit/5esunr-kwsr6c?file=src%2Fexample.tsx
Selecting a single row is noticeably slow and takes a while after the click to indicate to the user the row has been selected.

The issue is, selecting just a single row causes every other row to re-render as the parent component is re-rendering
Image

This is set up as the multiple select controlled example in the fluent v9 docs

Expected Behavior

Applying a couple changes to the code, I can ensure only the single row that is selected needs to be re-rendered
https://stackblitz.com/edit/5esunr-dk1pvn?file=src%2Fexample.tsx

The changes are

  1. Create a wrapper for DataGridRow that takes an item instead of a render function
  2. Memoize that component and use it instead of DataGridRow
  3. Extract selectionCell to a const so it doesn't get recreated every time

Image

Fixing this issue using the @fluentui-contrib/react-data-grid-react-window package took a little more work than just that as we need to

  1. Use a custom compare for the React.memo to handle the style prop changes
  2. Use a const that is created outside the component for DataGridBody.children so that function that returns the DataGridRow is constant

It would be nice if this example is changed to show the best practice to handle more rows, or there is a fix to enable better memoization without consumers having to build it themselves

There was a partial fix to this issue previously, but that only addressed uncontrolled selection (#29908)

Reproduction

https://stackblitz.com/edit/5esunr-kwsr6c?file=src%2Fexample.tsx

Steps to reproduce

  1. Go to stackblitz link
  2. Click on a row to select it
  3. Notice the delay before the row is actually selected

Are you reporting an Accessibility issue?

None

Suggested severity

Medium - Has workaround

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@spmonahan
Copy link
Contributor

@rocketBANG Thanks for all the details! Can you clarify: are you looking for code changes to DataGrid or documentation updates?

@rocketBANG
Copy link
Author

@spmonahan Ideally looking for a code change so consumers don't have to create their own version of DataGridRow and memorize that. But if that's not possible, then I think at least documentation updates so it's more obvious how to avoid this performance issue

@miroslavstastny
Copy link
Member

@ling1726 to check the profile to see if there is any optimization we can do on the library side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants