Perf: EuiDataGrid don't recreate columns on every change #2676
Conversation
| @@ -60,7 +60,7 @@ const columns = [ | |||
|
|
|||
| const raw_data = []; | |||
|
|
|||
| for (let i = 1; i < 100; i++) { | |||
| for (let i = 1; i < 500; i++) { | |||
There was a problem hiding this comment.
We do accept that kind of size in Kibana so maybe it's worth to add it here?
There was a problem hiding this comment.
You could always make this dynamic through state, and let the user define it in the docs. The reason I kept it low was that I cared more about the docs page being snappy, then checking the performance of the grid itself. But now that you've made these changes it's a less a concern.
There was a problem hiding this comment.
I think you're right! The initial load time doesn't change so it's worth to lower it. Thank you for this comment!
| columnId => | ||
| availableColumns.find( | ||
| ({ id }) => id === columnId | ||
| ) as EuiDataGridColumn // cast to avoid `undefined`, it filters those out next |
There was a problem hiding this comment.
I think (although I am not sure, I just wrapped it with useMemo but that's how I understand the comment) that the find returns nulls in case it won't find the id === columnId and after that we're getting rid of the nulls with filter 🤔
There was a problem hiding this comment.
Indeed, this looks like whitespace-only changes with the additional useMemo wrapping. I set originally set it up to filter second. No real reason, didn't occur to me to filter first 🤷♀
|
CI failure is a flaky test jenkins test this |
Summary
One thing I forgot to correct in my previous PR. The columns are being recreated on every change. That doesn't make the cells to update, but causes the update of rows. The correction makes a significant change for 500 rows table. (from 200ms to 20ms)
Checklist