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 potential memory leak warning #3558
Conversation
f9d1046
to
8828ace
Compare
{...errorProps} | ||
{...rootProps.componentsProps?.errorOverlay} | ||
/> | ||
<GridAutoSizer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes https://codesandbox.io/s/datagriddemo-material-demo-forked-49plk?file=/demo.tsx
If the error is set by the error
prop, GridAutoSizer
in GridBody
won't render and we won't have any dimension.
|
||
let height: React.CSSProperties['height'] = viewportInnerSize?.height ?? 0; | ||
if (rootProps.autoHeight && height === 0) { | ||
height = 'auto'; | ||
} | ||
|
||
if (!viewportInnerSize) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent misalignment when there's no dimension yet.
Fixes #3276
Preview: https://deploy-preview-3558--material-ui-x.netlify.app/components/data-grid/
I don't know why we never tried that. The warning is always triggered inside
GridOverlay
as part ofGridOverlays
. It turns out thatGridOverlays
is rendered beforeGridAutoSizer
. That way, subscribing inside a normal effect has no risk of losing any resize event, because React executes the effects sequentially according to the tree.