Skip to content

Commit

Permalink
Better ListView - FlatList
Browse files Browse the repository at this point in the history
Summary:
We really need a better list view - so here it is!

Main changes from existing `ListView`:

* Items are "virtualized" to limit memory - that is, items outside of the render window are unmounted and their memory is reclaimed. This means that instance state is not preserved when items scroll out of the render window.
* No `DataSource` - just a simple `data` prop of shape `Array<any>`. By default, they are expected to be of the shape `{key: string}` but a custom `rowExtractor` function can be provided for different shapes, e.g. graphql data where you want to map `id` to `key`. Note the underlying `VirtualizedList` is much more flexible.
* Fancy `scrollTo` functionality: `scrollToEnd`, `scrollToIndex`, and `scrollToItem` in addition to the normal `scrollToOffset`.
* Built-in pull to refresh support - set set the `onRefresh` and `refreshing` props.
* Rendering additional rows is usually done with low priority, after any interactions/animations complete, unless we're about to run out of rendered content. This should help apps feel more responsive.
* Component props replace render functions, e.g. `ItemComponent: ReactClass<{item: Item, index: number}>` replaces `renderRow: (...) => React.Element<*>`
* Supports dynamic items automatically by using `onLayout`, or `getItemLayout` can be provided for a perf boost and smoother `scrollToIndex` and scroll bar behavior.
* Visibility callback replaced with more powerful viewability callback and works in vertical and horizontal mode on at least Android and iOS, but probably other platforms as well. Extra power comes from the `viewablePercentThreshold` that lets the client decide when an item should be considered viewable.

Demo:

https://www.facebook.com/groups/576288835853049/permalink/753923058089625/

Reviewed By: yungsters

Differential Revision: D4412469

fbshipit-source-id: e2d891490bf76fe14df49294ecddf78a58adcf23
  • Loading branch information
sahrens authored and facebook-github-bot committed Feb 4, 2017
1 parent 57daad9 commit a345748
Show file tree
Hide file tree
Showing 12 changed files with 1,896 additions and 18 deletions.
166 changes: 166 additions & 0 deletions Examples/UIExplorer/js/FlatListExample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* The examples provided by Facebook are for non-commercial testing and
* evaluation purposes only.
*
* Facebook reserves all rights not expressly granted.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
* OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NON INFRINGEMENT. IN NO EVENT SHALL
* FACEBOOK BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*
* @flow
*/
'use strict';

const React = require('react');
const ReactNative = require('react-native');
const {
StyleSheet,
View,
} = ReactNative;

const FlatList = require('FlatList');
const UIExplorerPage = require('./UIExplorerPage');

const infoLog = require('infoLog');

const {
FooterComponent,
HeaderComponent,
ItemComponent,
PlainInput,
SeparatorComponent,
genItemData,
getItemLayout,
pressItem,
renderSmallSwitchOption,
} = require('./ListExampleShared');

class FlatListExample extends React.PureComponent {
static title = '<FlatList>';
static description = 'Performant, scrollable list of data.';

state = {
data: genItemData(1000),
horizontal: false,
filterText: '',
fixedHeight: true,
logViewable: false,
virtualized: true,
};
_onChangeFilterText = (filterText) => {
this.setState({filterText});
};
_onChangeScrollToIndex = (text) => {
this._listRef.scrollToIndex({viewPosition: 0.5, index: Number(text)});
};
render() {
const filterRegex = new RegExp(String(this.state.filterText), 'i');
const filter = (item) => (filterRegex.test(item.text) || filterRegex.test(item.title));
const filteredData = this.state.data.filter(filter);
return (
<UIExplorerPage
noSpacer={true}
noScroll={true}>
<View style={styles.searchRow}>
<PlainInput
onChangeText={this._onChangeFilterText}
placeholder="Search..."
value={this.state.filterText}
/>
<PlainInput
onChangeText={this._onChangeScrollToIndex}
placeholder="scrollToIndex..."
style={styles.searchTextInput}
/>
<View style={styles.options}>
{renderSmallSwitchOption(this, 'virtualized')}
{renderSmallSwitchOption(this, 'horizontal')}
{renderSmallSwitchOption(this, 'fixedHeight')}
{renderSmallSwitchOption(this, 'logViewable')}
</View>
</View>
<FlatList
HeaderComponent={HeaderComponent}
FooterComponent={FooterComponent}
ItemComponent={this._renderItemComponent}
SeparatorComponent={SeparatorComponent}
disableVirtualization={!this.state.virtualized}
getItemLayout={this.state.fixedHeight ? this._getItemLayout : undefined}
horizontal={this.state.horizontal}
data={filteredData}
key={(this.state.horizontal ? 'h' : 'v') + (this.state.fixedHeight ? 'f' : 'd')}
legacyImplementation={false}
onRefresh={() => alert('onRefresh: nothing to refresh :P')}
refreshing={false}
onViewableItemsChanged={this._onViewableItemsChanged}
ref={this._captureRef}
shouldItemUpdate={this._shouldItemUpdate}
/>
</UIExplorerPage>
);
}
_captureRef = (ref) => { this._listRef = ref; };
_getItemLayout = (data: any, index: number) => {
return getItemLayout(data, index, this.state.horizontal);
};
_renderItemComponent = ({item}) => {
return (
<ItemComponent
item={item}
horizontal={this.state.horizontal}
fixedHeight={this.state.fixedHeight}
onPress={this._pressItem}
/>
);
};
_shouldItemUpdate(prev, next) {
/**
* Note that this does not check state.horizontal or state.fixedheight because we blow away the
* whole list by changing the key in those cases. Make sure that you do the same in your code,
* or incorporate all relevant data into the item data, or skip this optimization entirely.
*/
return prev.item !== next.item;
}
// This is called when items change viewability by scrolling into or out of the viewable area.
_onViewableItemsChanged = (info: {
changed: Array<{
key: string, isViewable: boolean, item: any, index: ?number, section?: any
}>
}
) => {
// Impressions can be logged here
if (this.state.logViewable) {
infoLog('onViewableItemsChanged: ', info.changed.map((v) => ({...v, item: '...'})));
}
};
_pressItem = (key: number) => {
pressItem(this, key);
};
_listRef: FlatList;
}


const styles = StyleSheet.create({
options: {
flexDirection: 'row',
flexWrap: 'wrap',
alignItems: 'center',
},
searchRow: {
backgroundColor: '#eeeeee',
padding: 10,
},
});

module.exports = FlatListExample;
Loading

74 comments on commit a345748

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that FlatList does not and will never support sections or sticky headers. I'm working on a separate component, SectionList, which will though.

@ummahusla
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's looks great!

@bakso
Copy link

@bakso bakso commented on a345748 Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When run on device(iPhone 6), finger flipped screen fastly, scrollContent view blink blank some times very fast, maybe it is too fast to handle finish in limit case.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure what could be wrong, it takes less than a second to load on a 6-year-old android phone for me.

@bakso
Copy link

@bakso bakso commented on a345748 Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens And when flipped screen, could feel some frame was dropped still, not as smoothly as UITableView which you should garentee every cell is rendered in 16ms.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "flipped" do you mean scrolling really fast? More common terms are fling, flick, or hyperscroll.

Yes, blank content can get rendered when scrolling really fast, but frames shouldn't be dropping. What makes you think you're seeing frame drops? Also, UITable view can definitely drop frames - you can see it all the time in normal apps with complex content and not much time spent on optimizations. Entire frameworks like AsyncDisplayKit and ComponentKit have been built in part to combat those dropped frames.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perf optimization will be an ongoing project so it should only get better.

@bakso
Copy link

@bakso bakso commented on a345748 Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, some blank content doesn't lead to drop frame because async render combat those dropped frames.
I think the visual feeling of dropping frames maybe is another reason.
View maybe shook when scrolled, because the padding-top was not caculated accurately, but the value is changed frequently, so firstOffset was set two different value in different time, which leading to the shaking, which make the visual feeling of dropping frame.

<ScrollContentContainerViewClass>
<View key="$lead_spacer" style={{[!horizontal ? 'height' : 'width']: firstOffset}} />
</ScrollContentContainerViewClass>

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on a345748 Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens what about to add a refreshControlProps or refreshControlColor to support custom RefreshControl's color

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep the API slimmed down where possible. If you want to customize the refresh control, you can render your own with the renderScrollComponent prop.

@clauderic
Copy link

@clauderic clauderic commented on a345748 Feb 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sahrens, this is fantastic news 🙏 ❤️ 🎉

I had a chance to play with FlatList over the weekend, and after playing around with the windowSize and onEndReachedThreshold props, I have to say I was surprisingly impressed by the initial results. There was indeed a bit of dropped frames when flicking, but for the most part it was pretty darn smooth 👌 (https://twitter.com/clauderic_d/status/830603226584141824)

If you could summarize, what would you say are some of the main differences between this implementation and WindowedListView?

Also, what's the benefit of replacing render functions in favour of component props?

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's awesome, thanks for the feedback. I can definitely hook up onScroll.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The component props are more flexible - you can provide render functions like before (interpreted as functional components) or you can pass a component directly, like <FlatList data={data} ItemComponent={MyGenericListItem} />. Also note you don't have to set a key on each item like old ListView.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some of the main differences between this implementation and WindowedListView?

The biggest difference is versatility. WLV was built for one specific internal use-case and nothing else, so it didn't support lots of things, such as horizontal mode, separators, onRefresh, etc, and unintended uses were likely to surface bugs.

FlatList also has a new windowing algorithm that I think is better and will be improved even further.

The API is better I think, and the underlying implementation VirtualizedList is much more powerful, enabling things like dead-simple numColumns support for render grids, and a powerful SectionList component that is in the works. You can also use any form of input data you want, including immutable-js, by overriding the default getItem prop, etc.

@meno1
Copy link

@meno1 meno1 commented on a345748 Feb 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens this is great. I'm taking a look at FlatList. I see you can easily define getItemLayout. Does this only make a difference when disableVirtualization is false? I'm looking for a way to verify what I've defined for getItemLayout is correct/helping.

@cooperka
Copy link
Contributor

@cooperka cooperka commented on a345748 Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! I wrote up a simple migration guide on Medium if anyone wants to try it out. It includes links to multiple working example apps 👍

@jasongrishkoff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set this up on my side, and find that if I quickly scroll through a list, it gets quite choppy. Also not sure there's a onEndReached call? Not sure it'll work for infinite scroll.

@cooperka
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasongrishkoff if you pass an onEndReached callback to your list component it should be invoked here: a345748#diff-6e598319db0b64494c59910a5d5b5e24R439

@jasongrishkoff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cooperka Oh gosh, feeling dense! Thanks for that. Unfortunately the rapid increase/leak in memory on Android as I append more rows makes this incredibly unusable -- even moreso than ListView. This issue has become a bit of a show stopper for the release of my app.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's working fine in most cases so my guess is that you're doing something funny in your code if you're seeing what looks like a memory leak. Do you have any code to share?

@jasongrishkoff
Copy link

@jasongrishkoff jasongrishkoff commented on a345748 Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens Very possible. I tried creating a new React Native project and implementing the above example, but keep facing the attached issue.

@jasongrishkoff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I brought this up in #12512, but it's probably better suited here (and that thread was closed, as well).

I've got FlatList working using the very basic example provided in FlatListExample.js + ListExampleShared.js. No other modules or fancy things in my project -- it's a fresh install. The only thing I did was remove the thumbnails from the example. With this, there's still a pretty serious leak.

EXAMPLE 1 WITH VIRTUALIZED = TRUE AND fixedHeight = TRUE

Okay from there, it tends to hover around 132MB -- which is too high and will get killed by Android as soon as I started to use other apps (I'm on a Nexus 5X, so not a bad phone). Sure enough, I opened Gmail, then WhatsApp, and my AwesomeProject was cleaned/closed in the background.

Also note that scrolling was a bit choppy. I'd move down the feed and it'd sorta take a while to gather its positioning. Changing fixedHeight to false reduced that choppiness.

EXAMPLE 2 WITH VIRTUALIZED = TRUE AND fixedHeight = FALSE

So, in both cases it stopped hemorrhaging memory around the ~137MB mark. But again, that's way too high. Android seems to want to clear anything from memory taking up 80MB+ when they're doing their sweeps, and the additional +60MB that are added by a FlatList makes that impossible to avoid.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much memory do other apps take on that phone?

~130MB is pretty much where it will settle out based on the GC configuration and various buffers and caches, but a lot of that should get cleared if the app is backgrounded - are you seeing that?

@booboothefool
Copy link

@booboothefool booboothefool commented on a345748 Feb 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way I can get the current index / row for SeparatorComponent?

With a grid of 2 columns, I was previously using ListView renderSeparator rowID to show an ad after every 12 items:

[][]
[][]
[][]
[][]
[][]
[][]
[ad]
[][]
...
        renderSeparator={(sectionID, rowID, adjacentRowHighlighted) => {
          const CHUNK_SIZE = 12;
          const isLastItemInChunk = !((Number(rowID) + 1) % CHUNK_SIZE);
          const isLastItemWhenLessThanChunkSize = (hits.length < CHUNK_SIZE) && (Number(rowID) + 1 === hits.length);
          const shouldShowAd = isLastItemInChunk || isLastItemWhenLessThanChunkSize;
          if (shouldShowAd) {
            return (
              <View key={`ad_${sectionID},${rowID}`} style={styles.ad}>
                <AdMobBanner
                  bannerSize="mediumRectangle"
                  adUnitID={config.bannerUnitID}
                  testDeviceID="EMULATOR"
                  didFailToReceiveAdWithError={this.bannerError}
                />
              </View>
            );
          }
        }}

UPDATE: I actually noticed rendering any sort of separator lags a ton. I tried a simple one:

  _renderSeparatorComponent = () => {
    return (
      <Text>separator</Text>
    );
  }
   SeparatorComponent={this._renderSeparatorComponent}

and the difference in performance is like night and day (much smoother without a separator). The separator causes a bunch of flickering and stuttering when scrolling, specifically just as the scroll is about to end. Is there something else I have to configure?

@jasongrishkoff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens attached is a screenshot of memory usage. My app (com.decoderhq.indieshuffe) is right there up toward the top after a couple pages scrolled. Apps like Chrome and WhatsApp are quite a bit lower. I see very little reduction in memory usage when I background my React Native app (~5%).
memory

@meno1
Copy link

@meno1 meno1 commented on a345748 Feb 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried playing with windowSize, but it isn't clear how it's used. Also, I see a prop called viewablePercentThreshold, but I don't see this used anywhere in the code.

@booboothefool
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A screenshot of my app's memory using FlatList after scrolling ~1500 results:

screen shot 2017-02-26 at 2 12 35 am

@olofd
Copy link

@olofd olofd commented on a345748 Feb 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @sahrens . I only wish FB where a little transparent about their plans regarding lists in React Native. I was equally excited about a year ago when WindowedListView showed up, only to not be updated for 6 months. I hope this one gets the love it deserves (docs and leaving experimental)

A few questions:

  1. How does this compare to WindowedListView?
  2. Is this going to leave Experimental soon?

I know that these components are Experimental, but as I hope you understand that the performance of the original ListView (Memory-vice) makes many people start using these earlier then they maybe should.

EDIT: Saw your answer about how this compared to WindowedListView in an earlier comment. Sry for not reading.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, this will move out of the experimental folder, get proper docs, and will be fully supported and maintained pretty soon!

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still working out a couple breaking changes in the API which is why it hasn't been widely released yet.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things should be pretty stable now. I have a diff up internally to move these out of Experimental and the examples should be live in the UIExplorer. Main thing left is to add sticky header support then I think we can officialy mark ListView deprecated.

@booboothefool
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens What about the issues I am having at: #12512 (comment)

? Am I just doing something wrong?

@jasongrishkoff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try out the updated code today -- looks like you've pushed a few changes. However, I'm not optimistic as the numbers in the simple AwesomeProject test I did using the FlatListExample.js are way too high and will be cleared from background by Android as soon as another app is used. (see comment a345748#commitcomment-21030741 for details)

@olofd
Copy link

@olofd olofd commented on a345748 Mar 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some issues when I tried it as well (#12595). But these might be adressed at a later point?

@jasongrishkoff
Copy link

@jasongrishkoff jasongrishkoff commented on a345748 Mar 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the files, but now facing this error:

TaskQueue: Error with task : Tried to get frame for out of range index 0
_getFrameMetrics (VirtualizedList.js:625)
onUpdate (ViewabilityHelper.js:148)

I can dismiss that and carry on my merry way, but the memory leaks are too bad to handle. With each successive scroll the memory climbs and climbs (as shown in prior examples), and the app eventually becomes unresponsive (Android).

EDIT: I was able to get rid of this error by removing onViewableItemsChanged={this._onViewableItemsChanged}

@naqvitalha
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have actual recycling? On Android, destroying and recreating views frequently will cause excessive GC cycles and significantly effect battery.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the option to build recycling as an optimization under the hood, but it's unclear whether it's necessary. Because most of the rendering work is in JS, the java GC is much less burdened. We also have this experimental Nodes thing that significantly reduces the weight of native views and gets a solid perf boost. Nothing is off the table though, just a question of time and priority.

@olofd
Copy link

@olofd olofd commented on a345748 Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens From my experience. Building a lot of infinite or big lists in both react-native and pure native. Trying out ListView, WindowedListView and FlatList, I have to say that for react-native to be killer instead of great (Listview's imho are so important), some kind of recycling needs to happen. I really like the initiative here and admire the engineering going into this, but in reality. Even this suffers on a iPhone 5s. And I know, it's better on an iPhone 7. But my other natively based apps are so smooth on the 7 I notice a difference there as well. I would like, and have thought of building it myself, a react listview that uses recycling for these purposes (Simple list, lots of data) or even strays away from react and introduces some kind of templating that can be entirely rendered in native.

I still can't build an experience as apple's photos-app offers in terms of data vs smoothness (the bar might be high here :). But with the async model and two runtimes, this could be possible.

But I'll eagerly test all your work and rethink my opinions if needed.

Again. great work, love the simplification of the API here.

@LeoNatan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olofd In order to achieve smooth scrolling that supports fast scrolling and scroll to top, cells need to be provided and layout on the same runloop as the native list view request (cellForRowAtIndexPath:). The problem is in the RN render -> shadow -> yoga -> native loop. You have at least three runloop jumps (dispatch_async(dispatch_get_main_queue(), ...) as well as background thread work, which all work against the required goal. Precaching approaches have been proposed to mitigate this, but such approaches come with a plethora of caveats and limitations, and at the end of the day would still not solve the problem.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 4, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeoNatan
Copy link
Contributor

@LeoNatan LeoNatan commented on a345748 Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens At least as of current RN implementation, synchronizing all that seems very, very difficult, both due to possible deadlocks, data sharing and a large code refactor needed. But more fundamental issues are if the JS thread is busy. JS is single threaded, so any heavy work is done on the same thread. Synchronizing a UI thread with a thread that may be doing heavy lifting does not sound like a good idea at all. In a native project, it is understood that intensive operations are performed on background threads, while updating the main thread only when data is available. None of these mechanisms exit (async/await is not a multithreading mechanism), and I think the mindset of JS developers having to deal with multiple threads would be difficult to overcome.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 4, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naqvitalha
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens We actually built a separate renderScrollComponent for ListView based on native recycler view. It does actual recycling. Works extremely well as good as native, also added support to change from list to grid layouts on the fly. The code is not is best state though right now. Scrolling to offset doesn't work well. I intent to push it in but just cannot find the time. Works only on Android though but it's in production on Flipkart app with no issues.

@naqvitalha
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point that I'm trying to make is that all views being destroyed to reclaim memory are native objects in the end, which means all of them need to be GCed. Our benchmarks showed this approach to be unusable. Also, this will show up lot of blank spaces on fast scrolls.

@grundmanise
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens thanks for such a tremendous work!

Are there any FlatList methods that can stop scrolling and/or rendering of cells? Because while list is scrolling & rendering items the JS thread is blocked.. any method that can say to the FlatList "Hey, stop I've got a more important JS task to do"

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content rendered by FlatList while scrolling uses InteractionManager so if you want to pause it you just need to create an interaction handle.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naqvitalha: I would love to see how you did useful recycling - can you send a PR or Gist or something?

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have also used the native RecyclerView before but weren't able to get much better performance out of it.

@grundmanise
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens I see.. but still TouchableHighlight/Opacityis lagging when I use InteractionManager.

export default class ListItem extends Component { 

    shouldComponentUpdate() {
        return false;
    }

    render() {
        const {item} = this.props;

        return (
                <TouchableHighlight style={styles.item} underlayColor='rgba(255,255,255,.3)' activeOpacity={0.7} onPress={this._onPress}>
                     <View style={styles.row}>
                        <Image style={styles.image} source={{ uri: item.imgUrl}} />
                        <View style={styles.details}>
                            <BaseText style={styles.caption} weight={600}>{item.caption}</BaseText>
                            <BaseText style={styles.points}>{item.points} points</BaseText>
                        </View>
                        <TouchableOpacity hitSlop={{top: 10, left: 10, right: 10, bottom: 10}} >
                            <Icon style={styles.icon} name={item.favorite ? 'heart' : 'heart-outline'} size={24} color={'white'} />
                        </TouchableOpacity>
                    </View>
                </TouchableHighlight>
                )
    }

    _onPress = () => {
        const handle = InteractionManager.createInteractionHandle();
        this.props.onPress(this.props.item.key);
        InteractionManager.clearInteractionHandle(handle);
    }

}

@olofd
Copy link

@olofd olofd commented on a345748 Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the development of this list/a fast list in RN, somehow should be quantified in benchmarks.
I guess you have your own measuring @sahrens , but it would be great to make that public in some kind of clone-able repo.

It would be awesome to have solid number to work against instead of guesses and loose talk as these discussions tend to be.
What if there could be some kind of test-scenario set up and some kind of measuring pipe-line? (Memory usage, cells/rows per second (maybe automated scrolling of the list?) and so on) This way a fast and efficient list could be something all of the community could try to develop. And it would reward the most efficient solution instead of the most elegant one.

It would even show how other optimizations in RN affected the performance of the list.
Maybe to many variables? But it would be helpful and a lot of fun.

@sahrens
Copy link
Contributor Author

@sahrens sahrens commented on a345748 Mar 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great. It's definitely a little tricky to setup stuff like that, but not impossible, especially if intended for a human to run manually. I've been thinking about building a harness that calls scrollToEnd on a long list and tracks frame drops and how much blank content was rendered but haven't gotten around to it.

@desmond1121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ITxiansheng
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi,I made a custom refreshControll by listview‘s "renderScrollComponent" .If I migrate to Flatlist,how Can I implement this? I did not see the method in docs .https://facebook.github.io/react-native/releases/next/docs/flatlist.html @sahrens

@ptomasroos
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove onRefresh and pass it through a propp it will get passed down to VirtualizedList which then will render a scrollcomponent here
https://github.com/facebook/react-native/blob/master/Libraries/CustomComponents/Lists/VirtualizedList.js#L282 @ITxiansheng

@kralcifer-ms
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahrens I love what you did with FlatList and I have a couple of questions. I have a chat app that constantly has items being added to the list. The data for the list is bound as state.data and when I want to add messages I make a new array with what the current state has plus new messages. That causes the FlatList PureComponent to re-render. Your March 13, 2017 "Better List Views" post (which has some additional info from the current docs) mentions that it causes re-render to the fixed number of items in the "window". Is there a way to block the re-render on the existing items that haven't changed when I'm adding new messages to the list? My list items are a functional component which should get treated as PureComponent not get called if they didn't change but every list item in the window's render() is being called when adding one new item.

I'm trying to optimize for perf at the moment and I have some interesting Chrome bottom up timings for my FlatList that I thought you might be interested in seeing and share some insights with me. That topic is too much for a comment here I think.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure your functional components will get treated as PureComponent, should double check that. Should be independent of FlatList.

One thing you might also want to consider is the inverted prop combined with not actually adding new items to your chat list unless the user is scrolled to the bottom, and instead have a floating indicator to let the user know there are new messages (and tapping it would scroll to the bottom).

@kralcifer-ms
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expectation that if I have PureComponents as list items in a FlatList, that when I add a couple of new items to the list that the existing items in the "window" can avoid having their render() called? Setting a new copy of the list (the collection ref is different but the existing item refs are the same) is necessary to trigger the adds but FlatList doesn't dump every item in the window and make all new ones right?

I am using inverted. The primary use case though is to not be scrolled so the user would be at the spot where new messages are coming in and displayed alongside the previous ones.

@kralcifer-ms
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed from functional component to PureComponent and saw render only being called for the new adds.

@sahrens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, FlatList will render them just like if you rendered a bunch of items in a or

on web. React handles the re-rendering like any other component, respecting shouldComponentUpdate/PureComponent/etc. Except that it won't render items that are outside the render window at all - they are not mounted and the elements aren't even created.

Also note FlatList itself is a PureComponent so you have to change the data prop (or another prop) in order to trigger a re-render of the list itself.

I'm not sure what you mean by

The primary use case though is to not be scrolled so the user would be at the spot where new messages are coming in and displayed alongside the previous ones.
I'm suggesting that if you are having problems with things jumping around, you could try waiting to add the items to your list until the user has scrolled down far enough that they might actually see them. That may or may not be the UX you want though.

Please sign in to comment.