[Performance] All rows are re-rendered in the actual DOM whenever the application state changes

[ohana54]

Hi,

I discovered a major performance issue when I tried to use the grid in my application with about 1000 rows (max results set to 10, infinite scrolling enabled).
The rows are being re-rendered (in the actual DOM) whenever any state in the application changes.
This is due to a new unique key that's being generated every time a row is rendered, so react thinks it's a new row every time and replaces everything in the actual DOM.

A reproduction is shown here: http://jsfiddle.net/u0ue24ea/
Open the console and click on the "inc" button. I print to the console the DOM changes and you can see that all the rows are being replaced.

I believe this should be addressed ASAP if people are going to rely on this grid for their apps.

Thanks!

[FoxxMD]

I think this also prevents further customization of #75. When I render a custom row and then change a property of its state the row is immediately re-rendered with initial data, preventing any kind of internal interaction with the custom components.

[joellanciaux]

@ohana54 Thanks for putting together an issue. We added only rendering relevant rows infinite scrolling in master a little bit ago, but it hasn't made it's way to npm just yet. We're hoping to get this out with some other features/improvements in the near future, but you might be able to get moving with the source - I'll write up a little more on this later tonight.

This initially came about because we were a little reluctant to specify a row height when infinite scrolling, but I think that's the only way to make this happen.

[scvnc]

I have a hunch that the issue is usage of _.uniqueId for keying of components internally. I had similar problems with a different react grid doing the same thing. I was able to fix this when I removed usage of _.uniqueId in favor of requiring the user to define which property on the row object can uniquely identify a row. Otherwise every row was looking like a entirely different row since the keys get incremented every time react calls render()

After that it was able to properly diff and manipulate the DOM more efficiently. Using _.uniqueId may be a React anti-pattern.

[ohana54]

@joellanciaux I may have not been clear, the issue is present regardless of infinite scrolling. My example shows a basic grid without any fancy options. All the rows are re-rendered when any not grid related state changes. Again, I show it in the example.

@scvnc Yes, that is the root cause. I fixed it as well in the same way you did, but still experienced slow rendering (~250ms), which sent me looking for alternatives.

[scvnc]

I think PureRenderMixin might be the next piece for performance improvements. Now it's not doing all that unnecessary DOM manipulation right? So that time is probably spent recalculating the entire virtual DOM of the grid?

[joellanciaux]

@ohana54 Gotcha - @blkmutt put together something that takes care of a good bit, but I'll try looking into what's keeping things a little sluggish on large datasets (as you're running into). There's likely some 'shouldComponentUpdate' logic that needs to be added, or, as @scvnc mentioned, see how PureRenderMixin might fit into things.

@scvnc That's a good idea -- that might be a worthwhile addition. I'll be giving that a go a bit later (unless someone else beats me to it).

Semi-related, I do need to get some of these changes consolidated/documented a little bit more now that we have a few things that might belong in rowProperties (row height, etc.).

[FoxxMD]

Any update?

[anshumanf]

Fork of the JSFiddle provided by @ohana54 for anyone in an environment where the fb.me domain is blocked:

https://jsfiddle.net/anshumanf/3yem8mq1/

[conorhastings]

using unique ids as keys is considered an anti pattern ==> https://facebook.github.io/react/docs/reconciliation.html#trade-offs , particularly point 2.

[r1g0]

I had this same problem and since my table was simple I could manage with shouldUpdateComponent
https://facebook.github.io/react/docs/advanced-performance.html
Perhaps it works for someone else