Port over VirtualizedList from react-native

[tazsingh]

This patch solves the following problem
Hey everyone, can you give me some feedback on this?

I created #464 and can move forward with that but would like some feedback here first.

Since FlatList & VirtualizedList are JS implementations, they should be able to be ported over directly from React Native itself. That's what I've done here.

However what I've noticed is that the VirtualizedList performance is horrendously slow with the straight port. This makes me wonder if it's better to continue working on #464 as I know that the performance there is really good and can support all features over time, or to stick with this PR for a true port of the React Native functionality for compatibility but sacrificing performance?

Test plan

Test out #464 and this PR to let me know about performance. I've tested it on a list, and this PR is extremely slow to render initially and very janky during scrolling. But your experience may be different and I'd like to hear about it!

This pull request

• includes documentation
• includes tests
• includes benchmark reports
• includes an interactive example
• includes screenshots/videos

[brunolemos]

@tazsingh IMO api and final result must be exactly the same (no brainer drop in replacement, like the rest of this project), the underlying implementation not necessarily. Don't know necolas thoughts.

I would even highly consider react-virtualized for it.

[tazsingh]

I evaluated react-virtualized but you have to specify the dimensions upfront. This differs from what VirtualizedList does which is to approximate the dimensions in a lot of cases, unless it is provided upfront. This is part of the reason why I ventured down implementing a different List implementation that wasn't constrained by having dimensions defined upfront.

If you think react-virtualized can work here though, I'd love to hear about it rather than creating something new!

[RangerMauve]

@tazsingh You could get the dimensions by using onLayout on a View

[tazsingh]

@RangerMauve Are you suggesting render each and every item "off screen" with onLayout, wait for the initial render, then wait for onLayout to trigger, then once the dimensions are known put it into react-virtualized which would have to re-render the item again in its entirety based on the dimensions?

I'm not so sure that would be efficient, especially for use cases where Virtualized Lists are used (i.e. lots of data to render with performant scrolling).

Would be better to just dump the data in and let the List implementation deal with it, no? That's how VirtualizedList in React Native is doing it in the default case.

[RangerMauve]

Sorry, misunderstood. I thought react-virtualized only needed sizes for the container and not the children.

[tazsingh]

Ah all good. Yeah it handles its cells by essentially needing dimensions (or dimensions to fill) upfront. I'd love to be wrong here though if there is a better way! I don't want to implement new things if there's existing things that can work.

[RangerMauve]

I've had some luck using react-list for virtualization. Do its constraints seem more applicable here? I think it has an automatic way of dealing with variable sizes

[tazsingh]

@RangerMauve Ah cool, I passed that off to my team as they evaluated various libraries which led to me working on a new implementation as the others didn't fit our needs. Will gather their thoughts and take a look as well to see if it can work. Thanks!

[brunolemos]

I'm no expert in virtualization, but I know react-virtualized supports dynamic heights of content (on this demo, check Use dynamic row heights)

It's also the most popular solution on this field for react and I remember seeing @necolas and @bvaughn discussing this on twitter (link)

Some comments from them (react-native-web maintainer, react-virtualized maintainer and flatlist maintainer) here: #388

[tazsingh]

It's still requiring this height information upfront - https://github.com/bvaughn/react-virtualized/blob/5ef244538fdcf47fda8529d659f9001b81aafc16/source/List/List.example.js#L151

AFAIK, it won't let you render a cell with an undefined height which would be required for compat with React Native. Let me know if there's something that can work here though @bvaughn!

[bvaughn]

It's still requiring this height information upfront

AFAIK, it won't let you render a cell with an undefined height which would be required for compat with React Native. Let me know if there's something that can work here though @bvaughn!

@tazsingh CellMeasurer can be used for content that has unknown size upfront. It measures after rendering and caches measurements/adjusts positions.

May still need some tweaking because in the DOM, post-render-measurements can be done synchronously though.

[tazsingh]

@bvaughn Nice! Is there an example of this somewhere? Docs seem to suggest it's all needed upfront and I recall needing it when I was playing around with react-virtualized before.

Also can hop onto a chat room somewhere if it's more efficient. Thanks for helping out.

[brunolemos]

https://bvaughn.github.io/react-virtualized/#/components/CellMeasurer
https://github.com/bvaughn/react-virtualized/blob/master/source/CellMeasurer/CellMeasurer.example.js

[bvaughn]

Yeah, demo, demo source, docs. Admittedly the docs regarding CellMeasurer could be improved a bit.

[tazsingh]

I'll take another look at this with this info when I get a chance. Thanks!

Regardless of this, would like some feedback from @necolas (thanks @brunolemos for your feedback already) about porting the exact VirtualizedList logic from React Native vs implementing something new.

[necolas]

I'm interested in understanding what causes the straight port to be slow (last time I had a quick go at this it was rendering every item in the list; and there's an open question about how onLayout is currently implemented). Probably won't look at this properly until after React Europe

[tazsingh]

@necolas Good call, I'll look why it is slow and see if there's some improvements to be made there.

[jefferscn]

I use virtualizedlist ported from react-native , performance is acceptable , when i pass getItemLayout to control. I also test #464 , there seems to be some issues , and when i create a list with 2000 cells , my chrome crashed.

[tazsingh]

Back in town and working on this again. Gave the VirtualizedList a quick performance test from the Storybook example that I just committed.

Looks like it's not actually virtualizing the elements as the Node count within Chrome continues to grow. Also looks like getItemLayout produces unexpected results as I've noticed in this video

Happy to entertain suggestions on how to fix these things. Please provide real code examples of what's broken. But right now I'm going to switch over to giving react-virtualized a shot and will open yet another PR for that 😛

[jefferscn]

getItemLayout return the cell real size,so in the test case,renderItem function should return the size getItemLayout returns;

renderItem ({item, index}) {
    return <View style={{height:50}}><Text>{item.key} - {index}</Text></View>
  }

[jefferscn]

my change

• add keyExtractor
• set windowSize to 5
• set component(renderItem return)'s height to 50,equals with getItemsLyaout
• renderItem return PureComponent PureView

[tazsingh]

@jefferscn Thanks for your code. I was playing around with these options after posting my comment as well, but was initially expecting the returned item layout determine the layout of the item instead of the other way around.

I've narrowed down some of the jank that I'm noticing to a few lower level items within React Native Web. Now trying to optimize those functions to see if it can alleviate some of the issues I'm seeing.

Overall I don't think react-virtualized or #464 are the real solution here as the renderScrollComponent will kind of mess with their underlying virtualization. I think the best way forward is to work with this port and create an optimal experience for the web platform.

[jefferscn]

@tazsingh I agree with using this port as VirtualizedList. I'm doing some optimization as well.

[necolas]

#473 mentions issues with onLayout. Currently it involves iterating up the node tree, but it some cases that can be expensive if not unnecessary. It may be cheaper to return to using the bounding box API and something like getComputedStyle to determine if there is a scale transform that needs to be accounted for. Thanks for looking into optimizations

[necolas]

Do you want to rebase this, pick up any changes from upstream, and we can probably merge this into master and continue development of the lists from there?

[smkhalsa]

Is there anything I can do to help with this? @tazsingh are you still working on it?

[naqvitalha]

@Flipkart we've build a very well performing RecyclerView which supports staggered grid, works out of the box with ReactJS, also works with RNW in case you guys want to try. Solves our listview issues

[necolas]

Do you have a link to source code?

[naqvitalha]

@necolas it's still a few weeks away from being public.. I can get you access if you want to check it out..

[naqvitalha]

@necolas here you go https://www.npmjs.com/package/recyclerlistview, this is much faster than FlatList as of now

[necolas]

Thanks @naqvitalha. At first glance, it would be good to have unit tests and some optimizations for "non-determinisitic dimensions" (i.e., using direct manipulation rather than forcing a react re-render). I'm also open to looking at a PR that implements the React Native list components on top of this.

[naqvitalha]

Thanks for the suggestions @necolas. Tests and CI stuff will soon be added (lot needs to be done). We're using variable heights in one of the reviews page that we're building, has thousands of items. It actually does very well, no blanks or frame drops. I'll see if I can find time to build RN list on top of this. @Flipkart we chose not to use FlatList, blanks everywhere, worse on Android. So it might be a while before I take a shot at this.

[necolas]

Oh I see, I didn't realise FlatList has issues on Android. react-native bundles js-only list views but probably shouldn't. Nice work building a list view with performance you're happy with. For now I could link to your package and other cross-platform list packages in the Readme to help people find them.

I hope you don't mind me asking some questions :). Are you using this instead of the react-native lists in your native and web apps? What's the performance like when react-native-web is running the react-native version (compared to the web one)? What kind of abstractions have you built on top of this in your app? And have you tried using this to build "reverse lists" (as you would find in a chat app) and multicolumn lists? Thanks!

[naqvitalha]

Yes, usage in react-native in 100% but only recently started using in some places on the web properties. We're still building our first feature on RNW so don't have competing benchmarks but can share once we're done. Performance on web, however, is a challenge in general :(. Multicolumn lists work just fine since we support staggered grids, you can even have varyings widths. In our gridviews we have items with different row spans. No advance abstractions yet. Never tried reverse list but should be pretty easy, transform list by -180 and every item by +180. I can build a reverse mode in future, I think. Have done the same on Windows once.

[naqvitalha]

Also, most of FlatList issues in Android arise from the fact that you can scroll insanely fast. iOS scroll speed in relatively slower.

[naqvitalha]

@necolas almost forgot to ask.. What other cross platform lists are available? And yeah, a link in your project will help. Thanks for that!

[rmevans9]

@tazsingh I just did a merge of master into your branch locally and didn't have any merge conflicts other then the VirtaulizedList.js which was simply overcome by taking yours. @tazsingh if you don't have time or a want to update this PR let me know and I will open one with your changes on the latest master.

https://github.com/rmevans9/react-native-web/tree/port_virtualized_list

[dmueller39]

@rmevans9 @naqvitalha bump. I would appreciate it if someone moved this forward.

[rmevans9]

@dmueller39 sorry about my late reply. I will take a look at the branch I setup and make sure it is ready for a PR and open it. Should be able to do that within the next few days.

[Ahad-Wasim]

Has there been any update on this? There is a need for a Virtualized List for the project that I am working on. Please let me know if I could be of assistance in any way.

[naqvitalha]

@necolas actually writing FlatList/VirtualizedList using RecyclerListView should be pretty simple. May not have all features but should be easy.
@Ahad-Wasim If you need just virtualization you can try https://github.com/Flipkart/ReactEssentials, it is cross platform. If you specifically need VirtualizedList from RN you will need to write it.

[necolas]

Closing this as it's not moving forward and someone updated it over at #659. There are still several issues, but I don't yet have time to spend on it (Twitter doesn't use these components) and there is no clear maintainer for it either. I'd also like to see these JS components moved out of react-native core at some point.

[dmueller39]

@necolas I read the unimplemented view stubs as "not implemented yet". You might want to communicate what is on the roadmap for implementation vs what would need a maintainer to take on.