From 62c512eaddbb6875138ebdfcf43f6086e19b7786 Mon Sep 17 00:00:00 2001 From: Jason Gallavin Date: Thu, 12 Jul 2018 17:06:36 -0400 Subject: [PATCH 1/6] Remove unnecessary re-renders by replacing in-place lambda expression with a member function in VirtualList and modify the CellRenderer to pass its attributes back up. --- Libraries/Lists/VirtualizedList.js | 13 +++++---- .../Lists/__tests__/VirtualizedList-test.js | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 56364823f06d49..85ebf4506e5be5 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -688,7 +688,7 @@ class VirtualizedList extends React.PureComponent { key={key} prevCellKey={prevCellKey} onUpdateSeparators={this._onUpdateSeparators} - onLayout={e => this._onCellLayout(e, key, ii)} + onLayout={this._onCellLayout} onUnmount={this._onCellUnmount} parentProps={this.props} ref={ref => { @@ -1046,7 +1046,7 @@ class VirtualizedList extends React.PureComponent { } }; - _onCellLayout(e, cellKey, index) { + _onCellLayout = (e, cellKey, index): void => { const layout = e.nativeEvent.layout; const next = { offset: this._selectOffset(layout), @@ -1086,7 +1086,7 @@ class VirtualizedList extends React.PureComponent { } this._computeBlankness(); - } + }; _onCellUnmount = (cellKey: string) => { const curr = this._frames[cellKey]; @@ -1604,7 +1604,7 @@ class CellRenderer extends React.Component< index: number, inversionStyle: ?DangerouslyImpreciseStyleProp, 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, props: Object) => void, parentProps: { @@ -1670,6 +1670,9 @@ class CellRenderer extends React.Component< this.props.onUnmount(this.props.cellKey); } + _onLayout = (e): void => + this.props.onLayout(e, this.props.cellKey, this.props.index); + render() { const { CellRendererComponent, @@ -1694,7 +1697,7 @@ class CellRenderer extends React.Component< * comment and run Flow. */ getItemLayout && !parentProps.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 && ( diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js index 14aec9140f1195..1b94791cd11d3d 100644 --- a/Libraries/Lists/__tests__/VirtualizedList-test.js +++ b/Libraries/Lists/__tests__/VirtualizedList-test.js @@ -218,4 +218,33 @@ describe('VirtualizedList', () => { }), ); }); + + it('calls _onCellLayout properly', () => { + const items = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}]; + const mock = jest.fn(); + const component = ReactTestRenderer.create( + } + getItem={(data, index) => data[index]} + getItemCount={data => data.length} + />, + ); + const virtualList: VirtualizedList = component.getInstance(); + virtualList._onCellLayout = mock; + component.update( + } + 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); + }); }); From aab9d15dc66018b6da67ed1b24781e68327e717a Mon Sep 17 00:00:00 2001 From: Jason Gallavin Date: Fri, 13 Jul 2018 20:31:21 -0400 Subject: [PATCH 2/6] Make the CellRenderer a pure component. --- Libraries/Lists/VirtualizedList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 85ebf4506e5be5..93365944a8b87c 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -1594,7 +1594,7 @@ class VirtualizedList extends React.PureComponent { } } -class CellRenderer extends React.Component< +class CellRenderer extends React.PureComponent< { CellRendererComponent?: ?React.ComponentType, ItemSeparatorComponent: ?React.ComponentType<*>, From 3697edd5929d1139f60b8eac7e6a1c748ad270e7 Mon Sep 17 00:00:00 2001 From: Jason Gallavin Date: Sat, 14 Jul 2018 11:29:53 -0400 Subject: [PATCH 3/6] Change CellRenderer back from PureComponent to Component and implement shouldComponentUpdate. Apply shallowEqual on props excluding parentProps, shallowEqual on parentProps, and shallowEqual on state. --- Libraries/Lists/VirtualizedList.js | 77 +++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 93365944a8b87c..ea362a46053632 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -234,6 +234,65 @@ type ChildListState = { type State = {first: number, last: number}; +/** + * Performs equality by iterating through keys on an object and returning false + * when any key has values which are not strictly equal between the arguments. + * Returns true when the values of all keys are strictly equal. + */ +function shallowEqual(objA, objB, exceptions = []) { + if (is(objA, objB)) { + return true; + } + + if ( + typeof objA !== 'object' || + objA === null || + typeof objB !== 'object' || + objB === null + ) { + return false; + } + + var keysA = Object.keys(objA); + var keysB = Object.keys(objB); + + if (keysA.length !== keysB.length) { + return false; + } + + // Test for A's keys different from B. + for (let i = 0; i < keysA.length; i++) { + if (exceptions.findIndex(ex => ex === keysA[i]) !== -1) { + continue; + } + if ( + !hasOwnProperty.call(objB, keysA[i]) || + !is(objA[keysA[i]], objB[keysA[i]]) + ) { + return false; + } + } + + return true; +} + +/** + * inlined Object.is polyfill to avoid requiring consumers ship their own + * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is + */ +function is(x, y) { + // SameValue algorithm + if (x === y) { + // Steps 1-5, 7-10 + // Steps 6.b-6.e: +0 != -0 + // Added the nonzero y check to make Flow happy, but it is redundant + return x !== 0 || y !== 0 || 1 / x === 1 / y; + } else { + // Step 6.a: NaN == NaN + return x !== x && y !== y; + } +} + /** * Base implementation for the more convenient [``](/react-native/docs/flatlist.html) * and [``](/react-native/docs/sectionlist.html) components, which are also better @@ -675,6 +734,12 @@ class VirtualizedList extends React.PureComponent { if (stickyIndicesFromProps.has(ii + stickyOffset)) { stickyHeaderIndices.push(cells.length); } + // remove all components from parentProps so that these components don't cause the CellRenderer to re-render. + const parentProps = {...this.props}; + delete parentProps.ItemSeparatorComponent; + delete parentProps.ListEmptyComponent; + delete parentProps.ListFooterComponent; + delete parentProps.ListHeaderComponent; cells.push( { onUpdateSeparators={this._onUpdateSeparators} onLayout={this._onCellLayout} onUnmount={this._onCellUnmount} - parentProps={this.props} + parentProps={parentProps} ref={ref => { this._cellRefs[key] = ref; }} @@ -1673,6 +1738,16 @@ class CellRenderer extends React.PureComponent< _onLayout = (e): void => this.props.onLayout(e, this.props.cellKey, this.props.index); + shouldComponentUpdate(nextProps, nextState) { + if (!shallowEqual(this.props, nextProps, ['parentProps'])) { + return true; + } + if (!shallowEqual(this.props.parentProps, nextProps.parentProps)) { + return true; + } + return !shallowEqual(this.state, nextState); + } + render() { const { CellRendererComponent, From 8cf876fd690203ea0fbf0ae892407481553eafa1 Mon Sep 17 00:00:00 2001 From: Jason Gallavin Date: Sat, 14 Jul 2018 11:41:26 -0400 Subject: [PATCH 4/6] Change CellRenderer back from PureComponent to Component. --- Libraries/Lists/VirtualizedList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index ea362a46053632..9ec72c22aca0a6 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -1659,7 +1659,7 @@ class VirtualizedList extends React.PureComponent { } } -class CellRenderer extends React.PureComponent< +class CellRenderer extends React.Component< { CellRendererComponent?: ?React.ComponentType, ItemSeparatorComponent: ?React.ComponentType<*>, From ac4f4160b6aa072b1a0ad7ece941956de61ea3b0 Mon Sep 17 00:00:00 2001 From: JasonGallavin Date: Sat, 14 Jul 2018 12:28:11 -0400 Subject: [PATCH 5/6] Fix flow errors. --- Libraries/Lists/VirtualizedList.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 9ec72c22aca0a6..38dde5c0c15095 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -239,7 +239,7 @@ type State = {first: number, last: number}; * when any key has values which are not strictly equal between the arguments. * Returns true when the values of all keys are strictly equal. */ -function shallowEqual(objA, objB, exceptions = []) { +function shallowEqual(objA: Object, objB: Object, exceptions = []) { if (is(objA, objB)) { return true; } @@ -735,7 +735,7 @@ class VirtualizedList extends React.PureComponent { stickyHeaderIndices.push(cells.length); } // remove all components from parentProps so that these components don't cause the CellRenderer to re-render. - const parentProps = {...this.props}; + const parentProps: Props = {...this.props}; delete parentProps.ItemSeparatorComponent; delete parentProps.ListEmptyComponent; delete parentProps.ListFooterComponent; From 03e3eb4eaa8c305e4a9596642837417537f52f24 Mon Sep 17 00:00:00 2001 From: Jason Gallavin Date: Fri, 27 Jul 2018 22:36:21 -0400 Subject: [PATCH 6/6] Add onLayout guard. --- Libraries/Lists/VirtualizedList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 9ec72c22aca0a6..72ed40f5875cda 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -1736,6 +1736,7 @@ class CellRenderer extends React.Component< } _onLayout = (e): void => + this.props.onLayout && this.props.onLayout(e, this.props.cellKey, this.props.index); shouldComponentUpdate(nextProps, nextState) {