Permalink
Browse files

Kill shouldItemUpdate

Summary:
It was just adding unnecessary complexity. Users should just use standard React perf best practices, like `PureComponent` and `shouldComponentUpdate`.

This should be backwards compatible - existing `shouldItemUpdate` usage will just be ignored and should consider migrating to this pattern:

```
class MyItem extends React.PureComponent {
  _onPress = () => {
  	this.props.onPressItem(this.props.id);
  };
  render() {
    return (
      <SomeOtherWidget title={this.props.title} onPress={this._onPress} />
    )
  }
}
...
_renderItem = ({item}) => (
  <MyItem onPressItem={this._onPressItem} title={item.title} id={item.id} />
);
```

Which will automatically prevent re-renders unless the relavent data changes.

Reviewed By: yungsters

Differential Revision: D4730599

fbshipit-source-id: 0f61efe96eb4d95bb3b7c4ec889e3e0e34436e56
  • Loading branch information...
sahrens authored and facebook-github-bot committed Mar 22, 2017
1 parent 72670bf commit 5c856150ff66250e8d86ceb74d27740b49a5e06d
@@ -148,7 +148,6 @@ class FlatListExample extends React.PureComponent {
ref={this._captureRef}
refreshing={false}
renderItem={this._renderItemComponent}
shouldItemUpdate={this._shouldItemUpdate}
viewabilityConfig={VIEWABILITY_CONFIG}
/>
</UIExplorerPage>
@@ -169,15 +168,6 @@ class FlatListExample extends React.PureComponent {
/>
);
};
_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: {
@@ -107,7 +107,6 @@ class MultiColumnExample extends React.PureComponent {
onRefresh={() => alert('onRefresh: nothing to refresh :P')}
refreshing={false}
renderItem={this._renderItemComponent}
shouldItemUpdate={this._shouldItemUpdate}
disableVirtualization={!this.state.virtualized}
onViewableItemsChanged={this._onViewableItemsChanged}
legacyImplementation={false}
@@ -127,11 +126,6 @@ class MultiColumnExample extends React.PureComponent {
/>
);
};
_shouldItemUpdate(prev, next) {
// Note that this does not check state.fixedHeight because we blow away the whole list by
// changing the key anyway.
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<{
@@ -354,19 +354,6 @@ class FlatList<ItemT> extends React.PureComponent<DefaultProps, Props<ItemT>, vo
}
};
_shouldItemUpdate = (prev, next) => {
const {numColumns, shouldItemUpdate} = this.props;
if (numColumns > 1) {
return prev.item.length !== next.item.length ||
prev.item.some((prevItem, ii) => shouldItemUpdate(
{item: prevItem, index: prev.index + ii},
{item: next.item[ii], index: next.index + ii},
));
} else {
return shouldItemUpdate(prev, next);
}
};
render() {
if (this.props.legacyImplementation) {
return <MetroListView {...this.props} items={this.props.data} ref={this._captureRef} />;
@@ -379,7 +366,6 @@ class FlatList<ItemT> extends React.PureComponent<DefaultProps, Props<ItemT>, vo
getItemCount={this._getItemCount}
keyExtractor={this._keyExtractor}
ref={this._captureRef}
shouldItemUpdate={this._shouldItemUpdate}
onViewableItemsChanged={this.props.onViewableItemsChanged && this._onViewableItemsChanged}
/>
);
@@ -62,7 +62,6 @@ type NormalProps = {
refreshing?: boolean,
};
type DefaultProps = {
shouldItemUpdate: (curr: {item: Item}, next: {item: Item}) => boolean,
keyExtractor: (item: Item) => string,
};
type Props = NormalProps & DefaultProps;
@@ -90,7 +89,6 @@ class MetroListView extends React.Component {
);
}
static defaultProps: DefaultProps = {
shouldItemUpdate: () => true,
keyExtractor: (item, index) => item.key || index,
renderScrollComponent: (props: Props) => {
if (props.onRefresh) {
@@ -114,7 +112,7 @@ class MetroListView extends React.Component {
this.props,
{
ds: new ListView.DataSource({
rowHasChanged: (itemA, itemB) => this.props.shouldItemUpdate({item: itemA}, {item: itemB}),
rowHasChanged: (itemA, itemB) => true,
sectionHeaderHasChanged: () => true,
getSectionHeaderData: (dataBlob, sectionID) => this.state.sectionHeaderData[sectionID],
}),
@@ -109,13 +109,6 @@ type OptionalProps<SectionT: SectionBase<any>> = {
* Set this true while waiting for new data from a refresh.
*/
refreshing?: ?boolean,
/**
* This is an optional optimization to minimize re-rendering items.
*/
shouldItemUpdate: (
prevProps: {item: Item, index: number},
nextProps: {item: Item, index: number}
) => boolean,
/**
* Makes section headers stick to the top of the screen until the next one pushes it off. Only
* enabled by default on iOS because that is the platform standard there.
@@ -123,10 +123,6 @@ type OptionalProps = {
* Render a custom scroll component, e.g. with a differently styled `RefreshControl`.
*/
renderScrollComponent: (props: Object) => React.Element<any>,
shouldItemUpdate: (
props: {item: Item, index: number},
nextProps: {item: Item, index: number}
) => boolean,
/**
* Amount of time between low-pri item render batches, e.g. for rendering items quite a ways off
* screen. Similar fill rate/responsiveness tradeoff as `maxToRenderPerBatch`.
@@ -290,10 +286,6 @@ class VirtualizedList extends React.PureComponent<OptionalProps, Props, State> {
return <ScrollView {...props} />;
}
},
shouldItemUpdate: (
props: {item: Item, index: number},
nextProps: {item: Item, index: number},
) => true,
updateCellsBatchingPeriod: 50,
windowSize: 21, // multiples of length
};
@@ -624,9 +616,9 @@ class VirtualizedList extends React.PureComponent<OptionalProps, Props, State> {
if (dt > 500 && this._scrollMetrics.dt > 500 && (contentLength > (5 * visibleLength)) &&
!this._hasWarned.perf) {
infoLog(
'VirtualizedList: You have a large list that is slow to update - make sure ' +
'shouldItemUpdate is implemented effectively and consider getItemLayout, PureComponent, ' +
'etc.',
'VirtualizedList: You have a large list that is slow to update - make sure your ' +
'renderItem function renders components that follow React performance best practices ' +
'like PureComponent, shouldComponentUpdate, etc.',
{dt, prevDt: this._scrollMetrics.dt, contentLength},
);
this._hasWarned.perf = true;
@@ -760,20 +752,11 @@ class CellRenderer extends React.Component {
parentProps: {
renderItem: renderItemType,
getItemLayout?: ?Function,
shouldItemUpdate: (
props: {item: Item, index: number},
nextProps: {item: Item, index: number}
) => boolean,
},
};
componentWillUnmount() {
this.props.onUnmount(this.props.cellKey);
}
shouldComponentUpdate(nextProps, nextState) {
const curr = {item: this.props.item, index: this.props.index};
const next = {item: nextProps.item, index: nextProps.index};
return nextProps.parentProps.shouldItemUpdate(curr, next);
}
render() {
const {item, index, parentProps} = this.props;
const {renderItem, getItemLayout} = parentProps;
@@ -113,13 +113,6 @@ type OptionalProps<SectionT: SectionBase> = {
* Set this true while waiting for new data from a refresh.
*/
refreshing?: ?boolean,
/**
* This is an optional optimization to minimize re-rendering items.
*/
shouldItemUpdate: (
prevProps: {item: Item, index: number},
nextProps: {item: Item, index: number}
) => boolean,
};
export type Props<SectionT> =
@@ -249,14 +242,6 @@ class VirtualizedSectionList<SectionT: SectionBase>
return null;
}
_shouldItemUpdate = (prev, next) => {
const {shouldItemUpdate} = this.props;
if (!shouldItemUpdate || shouldItemUpdate(prev, next)) {
return true;
}
return this._getSeparatorComponent(prev.index) !== this._getSeparatorComponent(next.index);
}
_computeState(props: Props<SectionT>): State {
const offset = props.ListHeaderComponent ? 1 : 0;
const stickyHeaderIndices = [];
@@ -278,7 +263,6 @@ class VirtualizedSectionList<SectionT: SectionBase>
keyExtractor: this._keyExtractor,
onViewableItemsChanged:
props.onViewableItemsChanged ? this._onViewableItemsChanged : undefined,
shouldItemUpdate: this._shouldItemUpdate,
stickyHeaderIndices: props.stickySectionHeadersEnabled ? stickyHeaderIndices : undefined,
},
};
@@ -52,7 +52,6 @@ exports[`FlatList renders all the bells and whistles 1`] = `
renderItem={[Function]}
renderScrollComponent={[Function]}
scrollEventThrottle={50}
shouldItemUpdate={[Function]}
stickyHeaderIndices={Array []}
updateCellsBatchingPeriod={50}
windowSize={21}
@@ -145,7 +144,6 @@ exports[`FlatList renders empty list 1`] = `
renderItem={[Function]}
renderScrollComponent={[Function]}
scrollEventThrottle={50}
shouldItemUpdate={[Function]}
stickyHeaderIndices={Array []}
updateCellsBatchingPeriod={50}
windowSize={21}
@@ -176,7 +174,6 @@ exports[`FlatList renders null list 1`] = `
renderItem={[Function]}
renderScrollComponent={[Function]}
scrollEventThrottle={50}
shouldItemUpdate={[Function]}
stickyHeaderIndices={Array []}
updateCellsBatchingPeriod={50}
windowSize={21}
@@ -219,7 +216,6 @@ exports[`FlatList renders simple list 1`] = `
renderItem={[Function]}
renderScrollComponent={[Function]}
scrollEventThrottle={50}
shouldItemUpdate={[Function]}
stickyHeaderIndices={Array []}
updateCellsBatchingPeriod={50}
windowSize={21}
@@ -52,7 +52,6 @@ exports[`SectionList rendering empty section headers is fine 1`] = `
},
]
}
shouldItemUpdate={[Function]}
stickyHeaderIndices={
Array [
0,
@@ -179,7 +178,6 @@ exports[`SectionList renders all the bells and whistles 1`] = `
},
]
}
shouldItemUpdate={[Function]}
stickyHeaderIndices={
Array [
1,
@@ -282,7 +280,6 @@ exports[`SectionList renders empty list 1`] = `
renderScrollComponent={[Function]}
scrollEventThrottle={50}
sections={Array []}
shouldItemUpdate={[Function]}
stickyHeaderIndices={Array []}
stickySectionHeadersEnabled={true}
updateCellsBatchingPeriod={50}

0 comments on commit 5c85615

Please sign in to comment.