Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VirtualizedList CellRenderer optimization #21163

2 changes: 1 addition & 1 deletion Libraries/Components/ScrollView/ScrollViewStickyHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class ScrollViewStickyHeader extends React.Component<Props, State> {
this.props.onLayout(event);
const child = React.Children.only(this.props.children);
if (child.props.onLayout) {
child.props.onLayout(event);
child.props.onLayout(event, child.props.cellKey, child.props.index);
}
};

Expand Down
46 changes: 29 additions & 17 deletions Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,11 @@ class VirtualizedList extends React.PureComponent<Props, State> {
getItemCount,
horizontal,
keyExtractor,
getItemLayout,
renderItem,
ListItemComponent,
extraData,
debug,
} = this.props;
const stickyOffset = this.props.ListHeaderComponent ? 1 : 0;
const end = getItemCount(data) - 1;
Expand All @@ -715,9 +720,13 @@ class VirtualizedList extends React.PureComponent<Props, State> {
key={key}
prevCellKey={prevCellKey}
onUpdateSeparators={this._onUpdateSeparators}
onLayout={e => this._onCellLayout(e, key, ii)}
onLayout={this._onCellLayout}
onUnmount={this._onCellUnmount}
parentProps={this.props}
getItemLayout={getItemLayout}
renderItem={renderItem}
ListItemComponent={ListItemComponent}
extraData={extraData}
debug={debug}
ref={ref => {
this._cellRefs[key] = ref;
}}
Expand Down Expand Up @@ -1100,7 +1109,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
}
};

_onCellLayout(e, cellKey, index) {
_onCellLayout = (e, cellKey, index): void => {
const layout = e.nativeEvent.layout;
const next = {
offset: this._selectOffset(layout),
Expand Down Expand Up @@ -1141,7 +1150,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {

this._computeBlankness();
this._updateViewableItems(this.props.data);
}
};

_onCellUnmount = (cellKey: string) => {
const curr = this._frames[cellKey];
Expand Down Expand Up @@ -1676,15 +1685,14 @@ type CellRendererProps = {
index: number,
inversionStyle: ViewStyleProp,
item: Item,
onLayout: (event: Object) => void, // This is extracted by ScrollViewStickyHeader
onLayout: (event: Object, key: string, index: number) => void, // This is extracted by ScrollViewStickyHeader
onUnmount: (cellKey: string) => void,
onUpdateSeparators: (cellKeys: Array<?string>, props: Object) => void,
parentProps: {
getItemLayout?: ?Function,
renderItem?: ?RenderItemType<Item>,
ListItemComponent?: ?(React.ComponentType<any> | React.Element<any>),
},
prevCellKey: ?string,
getItemLayout?: ?Function,
renderItem: $PropertyType<OptionalProps, 'renderItem'>,
ListItemComponent?: ?(React.ComponentType<any> | React.Element<any>),
debug: ?boolean,
};

type CellRendererState = {
Expand Down Expand Up @@ -1765,6 +1773,10 @@ class CellRenderer extends React.Component<
this.props.onUnmount(this.props.cellKey);
}

_onLayout = (e): void =>
this.props.onLayout &&
this.props.onLayout(e, this.props.cellKey, this.props.index);

_renderElement(renderItem, ListItemComponent, item, index) {
if (renderItem && ListItemComponent) {
console.warn(
Expand Down Expand Up @@ -1799,14 +1811,17 @@ class CellRenderer extends React.Component<
const {
CellRendererComponent,
ItemSeparatorComponent,
ListItemComponent,
fillRateHelper,
horizontal,
item,
index,
inversionStyle,
parentProps,
renderItem,
getItemLayout,
debug,
} = this.props;
const {renderItem, getItemLayout, ListItemComponent} = parentProps;

const element = this._renderElement(
renderItem,
ListItemComponent,
Expand All @@ -1815,12 +1830,9 @@ class CellRenderer extends React.Component<
);

const onLayout =
/* $FlowFixMe(>=0.68.0 site=react_native_fb) This comment suppresses an
* error found when Flow v0.68 was deployed. To see the error delete this
* comment and run Flow. */
getItemLayout && !parentProps.debug && !fillRateHelper.enabled()
getItemLayout && !debug && !fillRateHelper.enabled()
? undefined
: this.props.onLayout;
: this._onLayout;
// NOTE: that when this is a sticky header, `onLayout` will get automatically extracted and
// called explicitly by `ScrollViewStickyHeader`.
const itemSeparator = ItemSeparatorComponent && (
Expand Down
55 changes: 55 additions & 0 deletions Libraries/Lists/__tests__/VirtualizedList-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,59 @@ describe('VirtualizedList', () => {
}),
);
});

it('calls _onCellLayout properly', () => {
const items = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];
const mock = jest.fn();
const component = ReactTestRenderer.create(
<VirtualizedList
data={items}
renderItem={({item}) => <item value={item.key} />}
getItem={(data, index) => data[index]}
getItemCount={data => data.length}
/>,
);
const virtualList: VirtualizedList = component.getInstance();
virtualList._onCellLayout = mock;
component.update(
<VirtualizedList
data={[...items, {key: 'i4'}]}
renderItem={({item}) => <item value={item.key} />}
getItem={(data, index) => data[index]}
getItemCount={data => data.length}
/>,
);
const cell = virtualList._cellRefs.i4;
const event = {
nativeEvent: {layout: {x: 0, y: 0, width: 50, height: 50}},
};
cell._onLayout(event);
expect(mock).toHaveBeenCalledWith(event, 'i4', 3);
});

it('handles extraData correctly', () => {
const mock = jest.fn();
const listData = [{key: 'i0'}, {key: 'i1'}, {key: 'i2'}];
const getItem = (data, index) => data[index];

Choose a reason for hiding this comment

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

no-shadow: 'data' is already declared in the upper scope.

const getItemCount = data => data.length;

Choose a reason for hiding this comment

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

no-shadow: 'data' is already declared in the upper scope.

const component = ReactTestRenderer.create(
<VirtualizedList
data={listData}
renderItem={mock}
getItem={getItem}
getItemCount={getItemCount}
/>,
);

component.update(
<VirtualizedList
data={listData}
renderItem={mock}
getItem={getItem}
getItemCount={getItemCount}
extraData={{updated: true}}
/>,
);
expect(mock).toHaveBeenCalledTimes(6);
});
});