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] Fix null ref when computeSizeAndPublishResizeEvent is called #8963

Closed
wants to merge 1 commit into from

Conversation

m4theushw
Copy link
Member

Alternative solution for #8697

@m4theushw m4theushw added the component: data grid This is the name of the generic UI component, not the React module! label May 11, 2023
@mui-bot
Copy link

mui-bot commented May 11, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-8963--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 608.6 996.6 691.5 765.7 154.783
Sort 100k rows ms 612.9 1,146.8 804.7 901.22 180.7
Select 100k rows ms 219.7 371 279.9 287.94 48.594
Deselect 100k rows ms 145.6 288.9 200 200.28 52.118

Generated by 🚫 dangerJS against 485cd51

Comment on lines -218 to -219
const rootEl = apiRef.current.rootElementRef?.current;
const mainEl = rootEl?.querySelector<HTMLDivElement>(`.${gridClasses.main}`);
Copy link
Member

Choose a reason for hiding this comment

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

I guess the main problem here is that the main element ref is assigned first, but we don't have direct access to it - therefore we use a query selector.
Instead of passing the element to this method, would it make more sense to make sure that the main element is accessible here directly?
I've opened #8968 with a unit test

Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of passing the element to this method, would it make more sense to make sure that the main element is accessible here directly?

Yes, it makes more sense but we'll only need to be more cautious now to not call computeSizeAndPublishResizeEvent before the ref is available. I'll close this PR.

@m4theushw m4theushw closed this May 12, 2023
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