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] Refactor: replace timeout with useTimeout #10179

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

romgrk
Copy link
Contributor

@romgrk romgrk commented Aug 31, 2023

Make use of useTimeout to declutter a bit.

NOTE: AUTO-MERGE ENABLED, only approve if this is good to go.

@romgrk romgrk added the component: data grid This is the name of the generic UI component, not the React module! label Aug 31, 2023
@romgrk romgrk enabled auto-merge (squash) August 31, 2023 01:24
@mui-bot
Copy link

mui-bot commented Aug 31, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-10179--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms -167.7 39.9 -95.4 -45.98 75.486
Sort 100k rows ms 646.3 1,499.3 1,429.2 1,259.96 312.593
Select 100k rows ms 706 1,051.5 718.7 812.74 125.586
Deselect 100k rows ms 122.9 279.1 207.3 204.98 49.705

Generated by 🚫 dangerJS against 44a9cc7

Comment on lines -136 to +135
[scrollDirection, apiRef],
[scrollDirection, apiRef, timeout],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing I don't like is that eslint doesn't recognize useTimeout as a React ref, so it requires this.

There is a react issue for this but it's been open forever: facebook/react#16873

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine, especially with the ESlint auto-fix turned on.

Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

👍🏻

@romgrk romgrk merged commit a8b474e into mui:master Aug 31, 2023
@cherniavskii cherniavskii mentioned this pull request Aug 31, 2023
7 tasks
@romgrk romgrk deleted the refactor-use-timeout-1 branch August 31, 2023 20:36
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!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants