-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[RFR] Fix Datagrid can't handle dynamic children #3635
Conversation
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.
As discussed with @djhi, the use of memo
here is useless as the children
prop will change at every render. Therefore, the performance is not really good. We must add an optimized
flag allowing to turn on speed optimizations when the columns of the datagrid don't change.
@fzaninotto I just tried this branch again with a CPU throttle set to x6 and I haven't notice any performance issues nor differences with the version without this fix. Can you try again ? |
@@ -91,7 +82,11 @@ DatagridBody.defaultProps = { | |||
row: <DatagridRow />, | |||
}; | |||
|
|||
const MemoDatagridBody = memo(DatagridBody); |
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.
memo doesn't work here as children
will always change
I think the default export should be DatagridBody, not MemoDatagridBody |
Fixes #3631 and #3629