React-virtualized: Statefull components in CellMeasurer

[kof]

Just stumbled over this issue: as CellMeasurer actually renders it's cells separately from the visual rendering in a hidden container with a clean initial state. However if a component's state has changed over time and this change modifies it's size, one needs to remeasure the cell with the current state, not the initial one.

[kof]

This issue shows actually a fundamental flow in the CellMeasurer logic. The idea we talked about some time ago (rendering a cell in the same container, just positioned outside of viewport and without 2x rendering - one for measuring and one for representation #341 ) won't have this issue.

[bvaughn]

Just stumbled over this issue: as CellMeasurer actually renders it's cells separately from the visual rendering in a hidden container with a clean initial state.

This is mentioned in the docs as a limitation of CellMeasurer for what it's worth.

However if a component's state has changed over time and this change modifies it's size, one needs to remeasure the cell with the current state, not the initial one.

This is why CellMeasurer passes reset methods to its child: resetMeasurementForColumn(index) and resetMeasurementForRow(index). If something changes in userland that invalids the cached measurement, CellMeasurer has no way to know and must be told to remeasure.

This issue shows actually a fundamental flow in the CellMeasurer logic. The idea we talked about some time ago (rendering a cell in the same container, just positioned outside of viewport and without 2x rendering - one for measuring and one for representation) won't have this issue.

The same issue would still exist as CellMeasurer would still _cache_ measurements so as to avoid constantly having to remeasure (which is expensive since it requires querying clientHeight and clientWidth).

Regardless, unfortunately, the idea you suggested- while reasonable- would require significant refactoring to accomplish and I don't have any ideas about how to tackle it (or the time to investigate it).

[bvaughn]

I don't think this is an actionable issue. The limitations of CellMeasurer's current approach are documented and clear-cache methods are provided for application code in the event of stateful components.

I think that's about as good as it gets for the time being.

[bvaughn]

I'd welcome a design proposal or a pull request (even just a proof-of-concept one) that shows a method of rendering a cell in the same container and quantifies the performance gains it offers. I don't have the bandwidth for this myself unfortunately.

[bvaughn]

Closed the issue for now since I don't plan on taking action on it. (CellMeasurer is "working as designed" for the moment.) But we can continue the discussion here. I'll still see and respond to comments. And as I said- I'd welcome a hand on prototyping the above suggestion if you're interested.

[kof]

I think you misunderstood the issue. The problem is that even when you reset heights cache, you are gonna render a component in its initial state for height measurement afterwards, if component is statefull.

[bvaughn]

I did indeed misunderstand that aspect of the issue. I thought when you were describing the "stateful component" you were talking about the containing/parent component having state that impacted its size or padding or something else style/display related.

[bvaughn]

I think the current generation of CellMeasurer can't support stateful components (at least not if the state has any impact on the rendered measurements).

[bvaughn]

I will add that to the documentation as well as another explicit warning.

[bvaughn]

To be honest though, stateful components within a virtualized list seems like a bad idea. As soon as they scroll out of the viewport, they'll be recycled and their state will be lost. For a user scrolling up and down- this will not be expected behavior. I think virtualized cells should always be pure, props-based for a good UX.

[kof]

I think the current generation of CellMeasurer can't support stateful components (at least not if the state has any impact on the rendered measurements).

Yeah well it can ... by cloning the visually rendered component (element) instead of rerendering, however this is still not solving the 2x performance overhead issue.

[bvaughn]

Once again, if you'd like to submit a PR for discussion- I'd welcome one. 😄

[kof]

To be honest though, stateful components within a virtualized list seems like a bad idea. As soon as they scroll out of the viewport, they'll be recycled and their state will be lost. For a user scrolling up and down- this will not be expected behavior. I think virtualized cells should always be pure, props-based for a good UX.

This is kinda opinionated, I see cases where this might be false.

[bvaughn]

I'm not really a good champion of CellMeasurer even though it falls to me by default. I have never used it in a production app (or anywhere outside of the demo site).

[bvaughn]

Sure, it's opinionated. It's my opinion. That's why I said they _seem_ like a bad idea. 😁

Still I think I have a pretty decent handle on UX and I think windowed lists should behave the same way as non-windowed lists to avoid surprising or confusing the user. So in that light I think stateful components are probably not a good idea.

[kof]

Once again, if you'd like to submit a PR for discussion- I'd welcome one. 😄

Yeah I know, I am just trying to document the issues. However if we always close issues which clearly need attention, the probability someone other than you is gonna fix them is low.

[kof]

Still I think I have a pretty decent handle on UX and I think windowed lists should behave the same way as non-windowed lists

hmm, not sure I am following ... if I have a non-windowed list, all changes in it are preserved when I scroll some items out of viewport and then come back. Thats actually the default behavior.

[bvaughn]

Hm. That's a fair point!

I don't close issues to be rude or anything. I'm just really concerned about letting things pile up, stress me out, and lead to burnout. (I look at projects like this as things to be cautious of.)

Maybe I'll open a sticky issue, similar to #147, that catalogs every issue that I'd welcome contributions on.

[bvaughn]

Still I think I have a pretty decent handle on UX and I think windowed lists should behave the same way as non-windowed lists

hmm, not sure I am following ... if I have a non-windowed list, all changes in it are preserved when I scroll out of viewport and then come back. Thats actually the default behavior.

Right. And so a windowed list should behave the same way. But with a stateful component- it would not. (Because the component goes away once it's scrolled off screen, and scrolling back would show a new instance- without the previous state.) This is why I think that stateful cells are a bad idea.

If you use props (eg state stored in redux or some other flux-like-thing) then scrolling back to the cell _would_ restore the previous "state" (and would also make it compatible with CellMeasurer).

[kof]

I look at projects like this as things to be cautious of.)

yeah that is crazy, I didn't read what type of issues are there, questions, feature requests etc. can be handled differently, however bugs and important enhancements which need to be implemented some day are better opened I think.

We just need to keep in mind that amount of issues shouln't make us crazy, also labels help to sort stuff out.

[kof]

Right. And so a windowed list should behave the same way. But with a stateful component- it would not. (Because the component goes away once it's scrolled off screen, and scrolling back would show a new instance- without the previous state.) This is why I think that stateful cells are a bad idea.

Thats true, it should work however at least as long as component is in viewport and survive updates.

[bvaughn]

A mix of all of the above is there.

I don't think I close "bugs" without fixing them (with the exception of one long-running Safari bug that I closed after months because I couldn't do anything about it). But I think ti's reasonable for me to close feature requests if I don't have the bandwidth to fix them because seeing a growing list of them every time I open the project is demotivational (at least for me, YMMV).

I am opening a sticky issue right now as a means of helping with that. I'll link to this issue (at least) so it isn't lost if someone is looking to contribute.

[bvaughn]

Bam: #373 :)

[bvaughn]

Thanks for expressing your concern about my closing feature requests. I hadn't thought of it that way before. Hopefully this sticky will help!

Feel free to ping me on any other issues you think I should add to it. 😄