-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[core] Simplify rows cache management #4933
Conversation
These are the results for the performance tests:
|
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.
Looks good to me but I never had to use the cache
packages/grid/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts
Outdated
Show resolved
Hide resolved
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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.
Much better with the values flattened.
getRowId: DataGridProcessedProps['getRowId']; | ||
loadingProp: DataGridProcessedProps['loading']; | ||
rows: GridRowsProp; |
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.
What do you think of making explicit that they are props? Pick
could also be used.
getRowId: DataGridProcessedProps['getRowId']; | |
loadingProp: DataGridProcessedProps['loading']; | |
rows: GridRowsProp; | |
getRowIdProp: DataGridProcessedProps['getRowId']; | |
loadingProp: DataGridProcessedProps['loading']; | |
rowsProp: DataGridProcessedProps['rows']; |
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.
Done
While working on #4927, I noticed that our cache structure had some complex legacy structure that was not useful anymore.
convertRowsPropToState
intocreateRowsInternalCache
createRowsInternalCache
that was basically "do nothing", only call it when we do want to regenerate the cachegridRowUtils
apiRef.current.unstable_caches.rows
non nullable, like for the state we can rely on the fact that the stat init is being done before the restGridRowsInternalCache
instead of having avalue
key storing almost everythingstate
(otherwise we can have misunderstandings between what leaves in the state and in the cache)