-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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 function optimization #20208
Changes from all commits
62c512e
aab9d15
3697edd
8cf876f
ac4f416
03e3eb4
f194e7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: Object, objB: Object, 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 [`<FlatList>`](/react-native/docs/flatlist.html) | ||
* and [`<SectionList>`](/react-native/docs/sectionlist.html) components, which are also better | ||
|
@@ -675,6 +734,12 @@ class VirtualizedList extends React.PureComponent<Props, State> { | |
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: Props = {...this.props}; | ||
delete parentProps.ItemSeparatorComponent; | ||
delete parentProps.ListEmptyComponent; | ||
delete parentProps.ListFooterComponent; | ||
delete parentProps.ListHeaderComponent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of copying the whole thing and then removing things you don't want to keep, can you explicitly copy just the props that we do want to keep? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In terms of perf, seems like copy whole thing and set not used property to null will be much better. https://jsperf.com/delete-vs-undefined-vs-null/16 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the original code the only parent props used by CellRenderer are |
||
cells.push( | ||
<CellRenderer | ||
CellRendererComponent={CellRendererComponent} | ||
|
@@ -688,9 +753,9 @@ 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} | ||
parentProps={parentProps} | ||
ref={ref => { | ||
this._cellRefs[key] = ref; | ||
}} | ||
|
@@ -1046,7 +1111,7 @@ class VirtualizedList extends React.PureComponent<Props, State> { | |
} | ||
}; | ||
|
||
_onCellLayout(e, cellKey, index) { | ||
_onCellLayout = (e, cellKey, index): void => { | ||
const layout = e.nativeEvent.layout; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe you should check index before actually use it.
If index is NaN, then _highestMeasuredFrameIndex will be also NaN, which may leads to crash. |
||
const next = { | ||
offset: this._selectOffset(layout), | ||
|
@@ -1086,7 +1151,7 @@ class VirtualizedList extends React.PureComponent<Props, State> { | |
} | ||
|
||
this._computeBlankness(); | ||
} | ||
}; | ||
|
||
_onCellUnmount = (cellKey: string) => { | ||
const curr = this._frames[cellKey]; | ||
|
@@ -1604,7 +1669,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<?string>, props: Object) => void, | ||
parentProps: { | ||
|
@@ -1670,6 +1735,20 @@ 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); | ||
|
||
shouldComponentUpdate(nextProps, nextState) { | ||
if (!shallowEqual(this.props, nextProps, ['parentProps'])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sahrens just to be clear, you are referring to deepDiffer |
||
return true; | ||
} | ||
if (!shallowEqual(this.props.parentProps, nextProps.parentProps)) { | ||
return true; | ||
} | ||
return !shallowEqual(this.state, nextState); | ||
} | ||
|
||
render() { | ||
const { | ||
CellRendererComponent, | ||
|
@@ -1694,7 +1773,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 && ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should comparing key length consider exceptions?