From 90baadb6ec5dd8719f9aaf01327b2b35512334a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Wed, 22 Aug 2018 15:36:22 +0200 Subject: [PATCH 01/12] Allow VirtualizedSectionList to handle any sort of data blob --- Libraries/Lists/SectionList.js | 8 +- Libraries/Lists/VirtualizedSectionList.js | 154 +++++++++++----------- 2 files changed, 86 insertions(+), 76 deletions(-) diff --git a/Libraries/Lists/SectionList.js b/Libraries/Lists/SectionList.js index 9d7d2eb8195d54..dd97cc0c58ee74 100644 --- a/Libraries/Lists/SectionList.js +++ b/Libraries/Lists/SectionList.js @@ -329,7 +329,13 @@ class SectionList> extends React.PureComponent< /* $FlowFixMe(>=0.66.0 site=react_native_fb) This comment suppresses an * error found when Flow v0.66 was deployed. To see the error delete this * comment and run Flow. */ - return ; + return items.length } + getItem={ (items, index) => items[index] } + getItemParam={ (item, param) => item[param] } + />; } _wrapperListRef: MetroListView | VirtualizedSectionList; diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index 616d976bbe12ef..0a3c78dc89128e 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -20,34 +20,24 @@ import type {Props as VirtualizedListProps} from 'VirtualizedList'; type Item = any; type SectionItem = any; +type Option = any; -type SectionBase = { - // Must be provided directly on each section. - data: $ReadOnlyArray, - key?: string, - - // Optional props will override list-wide props just for this section. - renderItem?: ?({ - item: SectionItem, - index: number, - section: SectionBase, - separators: { - highlight: () => void, - unhighlight: () => void, - updateProps: (select: 'leading' | 'trailing', newProps: Object) => void, - }, - }) => ?React.Element, - ItemSeparatorComponent?: ?React.ComponentType, - keyExtractor?: (item: SectionItem, index: ?number) => string, - - // TODO: support more optional/override props - // FooterComponent?: ?ReactClass, - // HeaderComponent?: ?ReactClass, - // onViewableItemsChanged?: ({viewableItems: Array, changed: Array}) => void, -}; +type SectionBase = any; type RequiredProps = { sections: $ReadOnlyArray, + /** + * A generic accessor for extracting an item from any sort of data blob. + */ + getItem: (data: any, index: number) => ?Item, + /** + * Determines how many items are in the data blob. + */ + getItemCount: (data: any) => number, + /** + * A generic accessor for extracting a section option from any sort of data blob. + */ + getItemParam: (section: any, name: string) => ?Option, }; type OptionalProps = { @@ -134,47 +124,32 @@ class VirtualizedSectionList extends React.PureComponent< Props, State, > { + static defaultProps: DefaultProps = { ...VirtualizedList.defaultProps, data: [], }; - scrollToLocation(params: { - animated?: ?boolean, - itemIndex: number, - sectionIndex: number, - viewPosition?: number, - }) { - let index = params.itemIndex + 1; - for (let ii = 0; ii < params.sectionIndex; ii++) { - index += this.props.sections[ii].data.length + 2; - } - const toIndexParams = { - ...params, - index, - }; - this._listRef.scrollToIndex(toIndexParams); - } - - getListRef(): VirtualizedList { - return this._listRef; - } - constructor(props: Props, context: Object) { super(props, context); this.state = this._computeState(props); } - UNSAFE_componentWillReceiveProps(nextProps: Props) { - this.setState(this._computeState(nextProps)); + static getDerivedStateFromProps( + nextProps: Props, + prevState: State, + ): ?State { + return this._computeState(nextProps); } _computeState(props: Props): State { const offset = props.ListHeaderComponent ? 1 : 0; const stickyHeaderIndices = []; const itemCount = props.sections.reduce((v, section) => { + const sectionData = props.getItemParam(section, 'data'); + stickyHeaderIndices.push(v + offset); - return v + section.data.length + 2; // Add two for the section header and footer. + return v + props.getItemCount(sectionData) + 2; // Add two for the section header and footer. }, 0); return { @@ -184,7 +159,7 @@ class VirtualizedSectionList extends React.PureComponent< ItemSeparatorComponent: undefined, // Rendered with renderItem data: props.sections, getItemCount: () => itemCount, - getItem, + getItem: (sections, index) => getItem(props, sections, index), keyExtractor: this._keyExtractor, onViewableItemsChanged: props.onViewableItemsChanged ? this._onViewableItemsChanged @@ -202,6 +177,30 @@ class VirtualizedSectionList extends React.PureComponent< ); } + scrollToLocation(params: { + animated?: ?boolean, + itemIndex: number, + sectionIndex: number, + viewPosition?: number, + }) { + let index = params.itemIndex + 1; + for (let ii = 0; ii < params.sectionIndex; ii++) { + const section = this.props.getItem(this.props.sections, ii); + const sectionData = this.props.getItemParam(section, 'data'); + const dataLength = this.props.getItemCount(sectionData); + index += dataLength + 2; + } + const toIndexParams = { + ...params, + index, + }; + this._listRef.scrollToIndex(toIndexParams); + } + + getListRef(): VirtualizedList { + return this._listRef; + } + _keyExtractor = (item: Item, index: number) => { const info = this._subExtractor(index); return (info && info.key) || String(index); @@ -221,38 +220,40 @@ class VirtualizedSectionList extends React.PureComponent< } { let itemIndex = index; const defaultKeyExtractor = this.props.keyExtractor; - for (let ii = 0; ii < this.props.sections.length; ii++) { - const section = this.props.sections[ii]; - const key = section.key || String(ii); + for (let ii = 0; ii < this.props.getItemCount(this.props.sections); ii++) { + const section = this.props.getItem(this.props.sections, ii); + const sectionData = this.props.getItemParam(section, 'data'); + const key = this.props.getItemParam(section, 'key') || String(ii); itemIndex -= 1; // The section adds an item for the header - if (itemIndex >= section.data.length + 1) { - itemIndex -= section.data.length + 1; // The section adds an item for the footer. + if (itemIndex >= this.props.getItemCount(sectionData) + 1) { + itemIndex -= this.props.getItemCount(sectionData) + 1; // The section adds an item for the footer. } else if (itemIndex === -1) { return { section, key: key + ':header', index: null, header: true, - trailingSection: this.props.sections[ii + 1], + trailingSection: this.props.getItem(this.props.sections, ii + 1), }; - } else if (itemIndex === section.data.length) { + } else if (itemIndex === this.props.getItemCount(sectionData)) { return { section, key: key + ':footer', index: null, header: false, - trailingSection: this.props.sections[ii + 1], + trailingSection: this.props.getItem(this.props.sections, ii + 1), }; } else { - const keyExtractor = section.keyExtractor || defaultKeyExtractor; + const keyExtractor = this.props.getItemParam(section, 'keyExtractor') || defaultKeyExtractor; + const sectionData = this.props.getItemParam(section, 'data'); return { section, - key: key + ':' + keyExtractor(section.data[itemIndex], itemIndex), + key: key + ':' + keyExtractor(this.props.getItem(sectionData, itemIndex), itemIndex), index: itemIndex, - leadingItem: section.data[itemIndex - 1], - leadingSection: this.props.sections[ii - 1], - trailingItem: section.data[itemIndex + 1], - trailingSection: this.props.sections[ii + 1], + leadingItem: this.props.getItem(sectionData, itemIndex - 1), + leadingSection: this.props.getItem(this.props.sections, ii - 1), + trailingItem: this.props.getItem(sectionData, itemIndex + 1), + trailingSection: this.props.getItem(this.props.sections, ii + 1), }; } } @@ -264,7 +265,7 @@ class VirtualizedSectionList extends React.PureComponent< if (!info) { return null; } - const keyExtractor = info.section.keyExtractor || this.props.keyExtractor; + const keyExtractor = this.props.getItemParam(info.section, 'keyExtractor') || this.props.keyExtractor; return { ...viewable, index: info.index, @@ -309,7 +310,7 @@ class VirtualizedSectionList extends React.PureComponent< return renderSectionFooter ? renderSectionFooter({section}) : null; } } else { - const renderItem = info.section.renderItem || this.props.renderItem; + const renderItem = this.props.getItemParam(info.section, 'renderItem') || this.props.renderItem; const SeparatorComponent = this._getSeparatorComponent(index, info); invariant(renderItem, 'no renderItem!'); return ( @@ -351,10 +352,11 @@ class VirtualizedSectionList extends React.PureComponent< return null; } const ItemSeparatorComponent = - info.section.ItemSeparatorComponent || this.props.ItemSeparatorComponent; + this.props.getItemParam(info.section, 'ItemSeparatorComponent') || this.props.ItemSeparatorComponent; const {SectionSeparatorComponent} = this.props; const isLastItemInList = index === this.state.childProps.getItemCount() - 1; - const isLastItemInSection = info.index === info.section.data.length - 1; + const sectionData = this.props.getItemParam(info.section, 'data'); + const isLastItemInSection = info.index === this.props.getItemCount(sectionData) - 1; if (SectionSeparatorComponent && isLastItemInSection) { return SectionSeparatorComponent; } @@ -441,7 +443,7 @@ class ItemWithSeparator extends React.Component< }, updateProps: (select: 'leading' | 'trailing', newProps: Object) => { const {LeadingSeparatorComponent, cellKey, prevCellKey} = this.props; - if (select === 'leading' && LeadingSeparatorComponent != null) { + if (select === 'leading' && LeadingSeparatorComponent !== null) { this.setState(state => ({ leadingSeparatorProps: {...state.leadingSeparatorProps, ...newProps}, })); @@ -516,22 +518,24 @@ class ItemWithSeparator extends React.Component< } } -function getItem(sections: ?$ReadOnlyArray, index: number): ?Item { +function getItem(props: Props, sections: ?$ReadOnlyArray, index: number): ?Item { if (!sections) { return null; } let itemIdx = index - 1; - for (let ii = 0; ii < sections.length; ii++) { - if (itemIdx === -1 || itemIdx === sections[ii].data.length) { + for (let ii = 0; ii < props.getItemCount(sections); ii++) { + const section = props.getItem(props.sections, ii); + const sectionData = props.getItemParam(section, 'data'); + if (itemIdx === -1 || itemIdx === props.getItemCount(sectionData)) { // We intend for there to be overflow by one on both ends of the list. // This will be for headers and footers. When returning a header or footer // item the section itself is the item. - return sections[ii]; - } else if (itemIdx < sections[ii].data.length) { + return section; + } else if (itemIdx < props.getItemCount(sectionData)) { // If we are in the bounds of the list's data then return the item. - return sections[ii].data[itemIdx]; + return props.getItem(sectionData, itemIdx); } else { - itemIdx -= sections[ii].data.length + 2; // Add two for the header and footer + itemIdx -= props.getItemCount(sectionData) + 2; // Add two for the header and footer } } return null; From cc92082048297eff68610a1655f31d3534abd974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Wed, 22 Aug 2018 15:36:38 +0200 Subject: [PATCH 02/12] Externalize VirtualizedSectionList --- Libraries/react-native/react-native-implementation.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Libraries/react-native/react-native-implementation.js b/Libraries/react-native/react-native-implementation.js index 3ff7b9ed3b495e..520bf945467cb6 100644 --- a/Libraries/react-native/react-native-implementation.js +++ b/Libraries/react-native/react-native-implementation.js @@ -153,6 +153,9 @@ const ReactNative = { get VirtualizedList() { return require('VirtualizedList'); }, + get VirtualizedSectionList() { + return require('VirtualizedSectionList'); + }, get WebView() { return require('WebView'); }, From 66c365a58730efde56761ef47e54e9bf760ec241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Wed, 22 Aug 2018 15:52:41 +0200 Subject: [PATCH 03/12] computeState use this, so keep using UNSAFE_componentWillReceiveProps --- Libraries/Lists/VirtualizedSectionList.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index 0a3c78dc89128e..00079a4cbed794 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -135,11 +135,8 @@ class VirtualizedSectionList extends React.PureComponent< this.state = this._computeState(props); } - static getDerivedStateFromProps( - nextProps: Props, - prevState: State, - ): ?State { - return this._computeState(nextProps); + UNSAFE_componentWillReceiveProps(nextProps: Props) { + this.setState(this._computeState(nextProps)); } _computeState(props: Props): State { From c00db10d0960fb68f78f23fbda8b452643707fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Wed, 22 Aug 2018 16:10:05 +0200 Subject: [PATCH 04/12] Fix flow error getItem function --- Libraries/Lists/VirtualizedSectionList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index 00079a4cbed794..df88f8d1b0474e 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -515,7 +515,7 @@ class ItemWithSeparator extends React.Component< } } -function getItem(props: Props, sections: ?$ReadOnlyArray, index: number): ?Item { +function getItem(props: Props, sections: ?$ReadOnlyArray, index: number): ?Item { if (!sections) { return null; } From 0762ef698cc7985381863a53b0d446db59927cba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Mon, 27 Aug 2018 13:00:10 +0200 Subject: [PATCH 05/12] Fix some errors --- Libraries/Lists/SectionList.js | 40 +- Libraries/Lists/VirtualizedSectionList.js | 53 +- .../__tests__/VirtualizedSectionList-test.js | 173 +++ .../__snapshots__/SectionList-test.js.snap | 5 + .../VirtualizedSectionList-test.js.snap | 1275 +++++++++++++++++ 5 files changed, 1517 insertions(+), 29 deletions(-) create mode 100644 Libraries/Lists/__tests__/VirtualizedSectionList-test.js create mode 100644 Libraries/Lists/__tests__/__snapshots__/VirtualizedSectionList-test.js.snap diff --git a/Libraries/Lists/SectionList.js b/Libraries/Lists/SectionList.js index dd97cc0c58ee74..11e7070bdda831 100644 --- a/Libraries/Lists/SectionList.js +++ b/Libraries/Lists/SectionList.js @@ -15,6 +15,8 @@ const React = require('React'); const ScrollView = require('ScrollView'); const VirtualizedSectionList = require('VirtualizedSectionList'); +const invariant = require('fbjs/lib/invariant'); + import type {ViewToken} from 'ViewabilityHelper'; import type {Props as VirtualizedSectionListProps} from 'VirtualizedSectionList'; @@ -322,6 +324,28 @@ class SectionList> extends React.PureComponent< } } + constructor(props: Props) { + super(props); + this._checkProps(this.props); + } + + componentDidUpdate(prevProps: Props) { + this._checkProps(this.props); + } + + _checkProps(props: Props) { + const { + getItem, + getItemCount, + getItemParam, + } = props; + invariant( + !getItem && !getItemCount && !getItemParam, + 'SectionList does not support custom data formats.', + ); + } + + render() { const List = this.props.legacyImplementation ? MetroListView @@ -329,13 +353,15 @@ class SectionList> extends React.PureComponent< /* $FlowFixMe(>=0.66.0 site=react_native_fb) This comment suppresses an * error found when Flow v0.66 was deployed. To see the error delete this * comment and run Flow. */ - return items.length } - getItem={ (items, index) => items[index] } - getItemParam={ (item, param) => item[param] } - />; + return ( + items.length} + getItem={(items, index) => items[index]} + getItemParam={(item, param) => item[param]} + /> + ); } _wrapperListRef: MetroListView | VirtualizedSectionList; diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index df88f8d1b0474e..83fbd9d9cf97cd 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -26,14 +26,6 @@ type SectionBase = any; type RequiredProps = { sections: $ReadOnlyArray, - /** - * A generic accessor for extracting an item from any sort of data blob. - */ - getItem: (data: any, index: number) => ?Item, - /** - * Determines how many items are in the data blob. - */ - getItemCount: (data: any) => number, /** * A generic accessor for extracting a section option from any sort of data blob. */ @@ -105,7 +97,8 @@ type OptionalProps = { */ refreshing?: ?boolean, }; - +/* $FlowFixMe - this Props seems to be missing a bunch of stuff. Remove this + * comment to see the errors */ export type Props = RequiredProps & OptionalProps & VirtualizedListProps; @@ -124,7 +117,6 @@ class VirtualizedSectionList extends React.PureComponent< Props, State, > { - static defaultProps: DefaultProps = { ...VirtualizedList.defaultProps, data: [], @@ -142,12 +134,14 @@ class VirtualizedSectionList extends React.PureComponent< _computeState(props: Props): State { const offset = props.ListHeaderComponent ? 1 : 0; const stickyHeaderIndices = []; - const itemCount = props.sections.reduce((v, section) => { - const sectionData = props.getItemParam(section, 'data'); + const itemCount = props.sections + ? props.sections.reduce((v, section) => { + const sectionData = props.getItemParam(section, 'data'); - stickyHeaderIndices.push(v + offset); - return v + props.getItemCount(sectionData) + 2; // Add two for the section header and footer. - }, 0); + stickyHeaderIndices.push(v + offset); + return v + props.getItemCount(sectionData) + 2; // Add two for the section header and footer. + }, 0) + : 0; return { childProps: { @@ -241,11 +235,16 @@ class VirtualizedSectionList extends React.PureComponent< trailingSection: this.props.getItem(this.props.sections, ii + 1), }; } else { - const keyExtractor = this.props.getItemParam(section, 'keyExtractor') || defaultKeyExtractor; + const keyExtractor = + this.props.getItemParam(section, 'keyExtractor') || + defaultKeyExtractor; const sectionData = this.props.getItemParam(section, 'data'); return { section, - key: key + ':' + keyExtractor(this.props.getItem(sectionData, itemIndex), itemIndex), + key: + key + + ':' + + keyExtractor(this.props.getItem(sectionData, itemIndex), itemIndex), index: itemIndex, leadingItem: this.props.getItem(sectionData, itemIndex - 1), leadingSection: this.props.getItem(this.props.sections, ii - 1), @@ -262,7 +261,9 @@ class VirtualizedSectionList extends React.PureComponent< if (!info) { return null; } - const keyExtractor = this.props.getItemParam(info.section, 'keyExtractor') || this.props.keyExtractor; + const keyExtractor = + this.props.getItemParam(info.section, 'keyExtractor') || + this.props.keyExtractor; return { ...viewable, index: info.index, @@ -307,7 +308,9 @@ class VirtualizedSectionList extends React.PureComponent< return renderSectionFooter ? renderSectionFooter({section}) : null; } } else { - const renderItem = this.props.getItemParam(info.section, 'renderItem') || this.props.renderItem; + const renderItem = + this.props.getItemParam(info.section, 'renderItem') || + this.props.renderItem; const SeparatorComponent = this._getSeparatorComponent(index, info); invariant(renderItem, 'no renderItem!'); return ( @@ -349,11 +352,13 @@ class VirtualizedSectionList extends React.PureComponent< return null; } const ItemSeparatorComponent = - this.props.getItemParam(info.section, 'ItemSeparatorComponent') || this.props.ItemSeparatorComponent; + this.props.getItemParam(info.section, 'ItemSeparatorComponent') || + this.props.ItemSeparatorComponent; const {SectionSeparatorComponent} = this.props; const isLastItemInList = index === this.state.childProps.getItemCount() - 1; const sectionData = this.props.getItemParam(info.section, 'data'); - const isLastItemInSection = info.index === this.props.getItemCount(sectionData) - 1; + const isLastItemInSection = + info.index === this.props.getItemCount(sectionData) - 1; if (SectionSeparatorComponent && isLastItemInSection) { return SectionSeparatorComponent; } @@ -515,7 +520,11 @@ class ItemWithSeparator extends React.Component< } } -function getItem(props: Props, sections: ?$ReadOnlyArray, index: number): ?Item { +function getItem( + props: Props, + sections: ?$ReadOnlyArray, + index: number, +): ?Item { if (!sections) { return null; } diff --git a/Libraries/Lists/__tests__/VirtualizedSectionList-test.js b/Libraries/Lists/__tests__/VirtualizedSectionList-test.js new file mode 100644 index 00000000000000..4b5318c135e317 --- /dev/null +++ b/Libraries/Lists/__tests__/VirtualizedSectionList-test.js @@ -0,0 +1,173 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * + * @format + * @emails oncall+react_native + */ +'use strict'; + +const React = require('React'); +const ReactTestRenderer = require('react-test-renderer'); + +const VirtualizedSectionList = require('VirtualizedSectionList'); + +describe('VirtualizedSectionList', () => { + it('renders simple list', () => { + const component = ReactTestRenderer.create( + } + getItem={(data, index) => data[index]} + getItemCount={data => data.length} + getItemParam={(data, name) => data[name]} + />, + ); + expect(component).toMatchSnapshot(); + }); + + it('renders empty list', () => { + const component = ReactTestRenderer.create( + } + getItem={(data, index) => data[index]} + getItemCount={data => data.length} + getItemParam={(data, name) => data[name]} + />, + ); + expect(component).toMatchSnapshot(); + }); + + it('renders null list', () => { + const component = ReactTestRenderer.create( + } + getItem={(data, index) => data[index]} + getItemCount={data => 0} + getItemParam={(data, name) => data[name]} + />, + ); + expect(component).toMatchSnapshot(); + }); + + it('renders empty list with empty component', () => { + const component = ReactTestRenderer.create( + } + ListFooterComponent={() =>