meliorence / react-native-snap-carousel

Swiper/carousel component for React Native featuring previews, multiple layouts, parallax images, performant handling of huge numbers of items, and more. Compatible with Android & iOS.
BSD 3-Clause "New" or "Revised" License
10.35k stars 2.29k forks source link

Replace FlatList with another component? #250

Open bd-arc opened 6 years ago

bd-arc commented 6 years ago

The FlatList component is just too buggy, period.

These plugins may constitute an interesting replacement:

bd-arc commented 6 years ago

Two major drawbacks:

PublicParadise commented 6 years ago

Frankly, I find FlatList unusable. My workaround for react-native-snap-carousel (which I love BTW) is setting initialNumToRender to data.length. That's the only way to get a bugfree version. This is a snippet from my post-install script:

# Stupid bug in flat list.
# @see comment in ListCarousel.tsx
sed -i.bak -e "s/initialNumToRender={initialNumToRender}/initialNumToRender={data.length}/" node_modules/react-native-snap-carousel/src/carousel/Carousel.js

Now, it would be nice if react-native-snap-carousel supported a feature that would allow me to set initialNumToRender from outside so I don't have to patch Carousel.js any longer. I would also argue that Carousel.js might want to set initialNumToRender to data.length by default if the expected number of items is under 100 (probably 90% of all use cases).

Of the two libraries that you found above I find react-native-largelist very intriguing. Would it be possible to work around the missing features, or perhaps convince the author to add those?

bd-arc commented 6 years ago

Hey @PublicParadise,

Well, initialNumToRender is part of the overridable props so you should be able to use <Carousel initialNumToRender={data.length} /> without any need for a patch ;-) I can guarantee that it works, since I've tried it in #235.

But if you have to do that and, therefore, are ready to forget about the performance optimizations that are supposed to come with FlatList, I would recommend to just set useScrollView to true. I've recently introduced this prop with this kind of use cases in mind and to completely bypass the utterly buggy FlatList. Moreover, it allows introducing awesome features like this one :p

Regarding the two plugins I'm considering, I need to run a lot of tests first. It would definitely help if we can convince the author of react-native-largelist to implement the missing yet required features!

Note that for the moment I don't feel confident basing my own plugin on a native one since I wouldn't be able to maintain it if the author stops doing so (whereas I would have no problem taking over a JS one).

PublicParadise commented 6 years ago

Hi @bd-arc,

thanks for the tips. To be honest I got so frustrated with FlatList that I placed the hack and never touched the component again. Back then initialNumToRender was not an overridable prop and useScrollView seems even better.

It seems that FlatList has some weird dependency to Animated. That's probably where most people have problems. Just 6 days ago somebody filed a new bug about that.

I myself was also looking into replacing FlatList and did some research. Windowing lists and rendering what's in view doesn't seem to be considered a hard problem. I liked the RxJS / most.js solutions the best.

bd-arc commented 6 years ago

Did you have any luck implementing one of those two solutions as a replacement?

PublicParadise commented 6 years ago

No, I only did the research. Back then I was still hoping that they would eventually fix FlatList. Let me share a few links from my research:

But I have a feeling that the native plugin (react-native-largelist) is the right way to go.

bd-arc commented 6 years ago

Thanks for sharing your research 👍

To be honest, I don't have time right now to put all this to the test. If you feel like taking a dive into react-native-largelist, that would be of tremendous help. If not, let's make sure to keep each other posted ;-)

PublicParadise commented 6 years ago

Update: as I was digging up those links I realized that Tal Kol from wix.com has a really good Medium blog. So far I like all of his articles. This one might also be relevant for our discussion:

PublicParadise commented 6 years ago

@bd-arc Same here, I have a pretty demanding day job and my time and resources are limited. I might look at the BindingListView. But yes: let's keep each other in the loop :)

bd-arc commented 6 years ago

Yes, Tal Kol has written some top-quality articles about React Native and performance optimization.

Also, I was particularly interested in Wix's plugin react-native-interactable, but unfortunately they haven't had time lately to tackle some pesky issues.

naqvitalha commented 6 years ago

@bd-arc What's the problem with recyclerlistview extending ScrollView? Both large-list and FlatList build upon the same. I wrote recyclerlistview and recently my team used this carousel component. We swapped FlatList with recyclerlistview and it works great.

bd-arc commented 6 years ago

Hey @naqvitalha, thanks for shining in!

From my experience, and until now, everything built upon a ScrollView ultimately shows performance limitations when there is a huge number of items to handle and/or feature limitations. But rest assured that my goal is to be proven wrong ;-)

Would you be interested in submitting a PR for this evolution so that it can be broadly and thoroughly tested?

naqvitalha commented 6 years ago

@bd-arc Sure. Let me do that.

amitassaraf commented 6 years ago

@naqvitalha @bd-arc Any status updates with the PR to replace flatlist with recyclerlistview? I would love to use it

0xpatrickdev commented 6 years ago

Unrelated to ScrollView FlatList and recyclerlistview, but has anyone looked into how react-native-gesture-handler might be able to improve this lib?

From the docs:

With this library gestures are no longer controlled by the JS responder system, but instead are recognized and tracked in the UI thread. It makes touch interactions and gesture tracking not only smooth, but also dependable and deterministic.

It requires react-native link, which is unfortunate in terms of keeping this lib dependency free. But it is now included in Expo/CRNA.

bd-arc commented 6 years ago

Hey @pcooney10,

I've previously considered implementing a custom PanResponder on top of the ScrollView/FlatList one (as you can see in #40).

Have you tried something similar with react-native-gesture-handler? I'd love to get some feedback in order to determine if this is a legitimate idea or a straight way to madness...

naqvitalha commented 6 years ago

@amitassaraf Contracts of this component are very similar to FlatList since props are passed down. For RLV layout provider is mandatory. It'll be a breaking change or we'll need to introduce a new mode.

machester4 commented 6 years ago

Hello everyone, I want to share a little of my experience using this component. I needed to use it within ScrollView which generated the problem that all the items will be shown by more than you would indicate the property removeClippedSubviews the elements are not deleted from the memory, otherwise, each path created new elements causing the app to drain the RAM memory in Android To obtain a correct operation of the list within a ScrollView (or another list) use the following FlatList properties.

maxToRenderPerBatch={4}

initialNumToRender={4}

windowSize={4}

removeClippedSubviews={Platform.OS != 'ios'}

Depending on your case, you can configure the values ​​that best suit your needs. in this way you will have the expected performance with almost no memory leak

bd-arc commented 6 years ago

@machester4 Thanks for sharing your findings! Those values won't fit everyone's use case, but the approach is sound ;-)

machester4 commented 6 years ago

You can apply that logic to your horizontal lists that are within a vertical list. in my case all my horizontal lists have 3 elements visible at a time. for that reason the values ​​are of maxToRenderPerBatchand initialNumToRenderare in 4 which is where I get a better experience for the user.

srameshr commented 6 years ago

@naqvitalha Could you share how you replaced FlatList with RecylerView inside carousel? Also, do you support something like https://github.com/facebook/react-native/issues/20500 in RecyclerView?

PublicParadise commented 6 years ago

@bd-arc Heads up, I am pretty sure this will affect react-native-snap-carousel clients: https://github.com/facebook/react-native/issues/21070

bd-arc commented 6 years ago

Thanks for the heads up @PublicParadise!

FlatList never stops to amaze... It looks like an InteractionManager might be running somewhere, thus postponing the component logic's execution.

It's definitely time to get rid of FlatList for good :-)

Maclay74 commented 5 years ago

Hi, any updates on this field so far?

danielgindi commented 5 years ago

Actually FlatList in my opinion has what it takes to become a really great list component. All that it's missing is the concept of "estimated row height" that UITableView has on iOS. So you DO NOT need a property for "how many rows should I layout asynchronously to do some hackery and trickery".

All we need is, as I said, an estimatedRowHeight (possibly with callback to allow different estimates per row, but still estimates!).

So content size will be changing dynamically but nobody will care about it or feel it, as you could scroll to last item or any other index and it will get there without glitches, and you won't be doing so much work that there will be missing items while scrolling.

sebastianestrella commented 5 years ago

Hi @naqvitalha, Do you have some branch or PR about to migrate flatlist to RLV for this component? It will be very useful for me. Thanks.

ATShiTou commented 5 years ago

Thank you for trying my component.

For almost all scenes, you should do it like this:

<Carousel  containerCustomStyle={{flex:1}} contentContainerCustomStyle={{flex:1}} renderItem={()=><LargeList ...props/>}/>

And ensure all the parents of Carousel contains {flex:1} style.

Pay attention to this tip:

LargeList default has a {flex:1} style,please be sure its parent has a bounded height.

LargeList can not work well if you want its items prop up LargeList's size. You should confirm your LargeList size is inherited from its parent or a bounded height.

Forgive me for my pool English, if you understand Chinese. Check out this issue

greyhands2 commented 5 years ago

The FlatList component is just too buggy, period.

These plugins may constitute an interesting replacement:

* https://github.com/Flipkart/recyclerlistview

* https://github.com/bolan9999/react-native-largelist

well said

dohooo commented 3 years ago

Sorry, please allow me to advertise for my open source library! ~ I think this library react-native-reanimated-carousel will solve your problem. It is a high performance and very simple component, complete with React-Native reanimated 2