From 2a1ab362572cf7bc4aaa1982e3f5aa43ea6e1870 Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Tue, 28 Feb 2017 02:09:09 -0800 Subject: [PATCH] Breaking API change - abandon `ItemComponent` in favor of `renderItem` Summary: After a fair bit of use, we have concluded that the `ItemComponent` mechanism is not worth the hassle. Flow has trouble type checking it thoroughly, requiring an 'item' prop is annoying, and it is very common to need to capture `this` anyway, e.g. for an `onPress` handler. A common pattern was something like: _renderItem = ({item}) => this._onPress(item)} />}; ... ItemComponent={this._renderItem} which wouldn't flow check the props and doesn't benefit from reusing components. If we find some specific patterns that would benefit from the `ItemComponent` pattern, we can create a new component that provides that API and wraps `FlatList` under the hood. I'm going to do `SectionList` in a stacked diff. Reviewed By: bvaughn Differential Revision: D4625338 fbshipit-source-id: a4901f1c9d77e0115b0b8032b8c210f624e97ea3 --- Examples/UIExplorer/js/FlatListExample.js | 2 +- Examples/UIExplorer/js/ListExampleShared.js | 28 +++---- Examples/UIExplorer/js/MultiColumnExample.js | 2 +- Examples/UIExplorer/js/SectionListExample.js | 10 +-- Libraries/Experimental/FlatList.js | 42 +++++++---- Libraries/Experimental/MetroListView.js | 15 ++-- Libraries/Experimental/SectionList.js | 14 ++-- Libraries/Experimental/VirtualizedList.js | 11 +-- .../Experimental/VirtualizedSectionList.js | 38 +++++----- .../__flowtests__/FlatList-flowtest.js | 73 +++++++++++-------- .../__flowtests__/SectionList-flowtest.js | 62 +++++++--------- 11 files changed, 154 insertions(+), 143 deletions(-) diff --git a/Examples/UIExplorer/js/FlatListExample.js b/Examples/UIExplorer/js/FlatListExample.js index 502e1d4d2..0871e7374 100644 --- a/Examples/UIExplorer/js/FlatListExample.js +++ b/Examples/UIExplorer/js/FlatListExample.js @@ -108,7 +108,6 @@ class FlatListExample extends React.PureComponent { diff --git a/Examples/UIExplorer/js/ListExampleShared.js b/Examples/UIExplorer/js/ListExampleShared.js index 173fe3f77..d4a870cdd 100644 --- a/Examples/UIExplorer/js/ListExampleShared.js +++ b/Examples/UIExplorer/js/ListExampleShared.js @@ -86,22 +86,16 @@ class ItemComponent extends React.PureComponent { } } -class StackedItemComponent extends React.PureComponent { - props: { - item: Item, - }; - render() { - const {item} = this.props; - const itemHash = Math.abs(hashCode(item.title)); - const imgSource = THUMB_URLS[itemHash % THUMB_URLS.length]; - return ( - - {item.title} - {item.text} - - - ); - } -} +const renderStackedItem = ({item}: {item: Item}) => { + const itemHash = Math.abs(hashCode(item.title)); + const imgSource = THUMB_URLS[itemHash % THUMB_URLS.length]; + return ( + + {item.title} - {item.text} + + + ); +}; class FooterComponent extends React.PureComponent { render() { @@ -287,9 +281,9 @@ module.exports = { ItemComponent, PlainInput, SeparatorComponent, - StackedItemComponent, genItemData, getItemLayout, pressItem, renderSmallSwitchOption, + renderStackedItem, }; diff --git a/Examples/UIExplorer/js/MultiColumnExample.js b/Examples/UIExplorer/js/MultiColumnExample.js index 8ec0ff590..d682c2d41 100644 --- a/Examples/UIExplorer/js/MultiColumnExample.js +++ b/Examples/UIExplorer/js/MultiColumnExample.js @@ -99,7 +99,6 @@ class MultiColumnExample extends React.PureComponent { alert('onRefresh: nothing to refresh :P')} refreshing={false} + renderItem={this._renderItemComponent} shouldItemUpdate={this._shouldItemUpdate} disableVirtualization={!this.state.virtualized} onViewableItemsChanged={this._onViewableItemsChanged} diff --git a/Examples/UIExplorer/js/SectionListExample.js b/Examples/UIExplorer/js/SectionListExample.js index 11a62adca..41d3279f7 100644 --- a/Examples/UIExplorer/js/SectionListExample.js +++ b/Examples/UIExplorer/js/SectionListExample.js @@ -42,10 +42,10 @@ const { ItemComponent, PlainInput, SeparatorComponent, - StackedItemComponent, genItemData, pressItem, renderSmallSwitchOption, + renderStackedItem, } = require('./ListExampleShared'); const VIEWABILITY_CONFIG = { @@ -54,7 +54,7 @@ const VIEWABILITY_CONFIG = { waitForInteraction: true, }; -const SectionHeaderComponent = ({section}) => ( +const renderSectionHeader = ({section}) => ( SECTION HEADER: {section.key} @@ -104,16 +104,16 @@ class SectionListExample extends React.PureComponent { } ItemSeparatorComponent={() => } enableVirtualization={this.state.virtualized} onRefresh={() => alert('onRefresh: nothing to refresh :P')} onViewableItemsChanged={this._onViewableItemsChanged} refreshing={false} + renderItem={this._renderItemComponent} + renderSectionHeader={renderSectionHeader} sections={[ - {ItemComponent: StackedItemComponent, key: 's1', data: [ + {renderItem: renderStackedItem, key: 's1', data: [ {title: 'Item In Header Section', text: 'Section s1', key: '0'}, ]}, {key: 's2', data: [ diff --git a/Libraries/Experimental/FlatList.js b/Libraries/Experimental/FlatList.js index 99c1b880f..240f0c73b 100644 --- a/Libraries/Experimental/FlatList.js +++ b/Libraries/Experimental/FlatList.js @@ -43,14 +43,21 @@ import type {StyleObj} from 'StyleSheetTypes'; import type {ViewabilityConfig, ViewToken} from 'ViewabilityHelper'; import type {Props as VirtualizedListProps} from 'VirtualizedList'; -type Item = any; - type RequiredProps = { /** - * Note this can be a normal class component, or a functional component, such as a render method - * on your main component. + * Takes an item from `data` and renders it into the list. Typicaly usage: + * + * _renderItem = ({item}) => ( + * this._onPress(item)}> + * {item.title}} + * + * ); + * ... + * + * + * Provides additional metadata like `index` if you need it. */ - ItemComponent: ReactClass<{item: ItemT, index: number}>, + renderItem: ({item: ItemT, index: number}) => ?React.Element<*>, /** * For simplicity, data is just a plain array. If you want to use something else, like an * immutable list, use the underlying `VirtualizedList` directly. @@ -125,8 +132,8 @@ type OptionalProps = { * Optional optimization to minimize re-rendering items. */ shouldItemUpdate: ( - prevProps: {item: ItemT, index: number}, - nextProps: {item: ItemT, index: number} + prevInfo: {item: ItemT, index: number}, + nextInfo: {item: ItemT, index: number} ) => boolean, /** * See ViewabilityHelper for flow type and comments. @@ -159,7 +166,7 @@ type DefaultProps = typeof defaultProps; * * {item.key}} + * renderItem={({item}) => {item.key}} * /> */ class FlatList extends React.PureComponent, void> { @@ -186,7 +193,7 @@ class FlatList extends React.PureComponent, vo * Requires linear scan through data - use scrollToIndex instead if possible. May be janky without * `getItemLayout` prop. */ - scrollToItem(params: {animated?: ?boolean, item: Item, viewPosition?: number}) { + scrollToItem(params: {animated?: ?boolean, item: ItemT, viewPosition?: number}) { this._listRef.scrollToItem(params); } @@ -305,18 +312,21 @@ class FlatList extends React.PureComponent, vo } }; - _renderItem = ({item, index}) => { - const {ItemComponent, numColumns, columnWrapperStyle} = this.props; + _renderItem = (info: {item: ItemT | Array, index: number}) => { + const {renderItem, numColumns, columnWrapperStyle} = this.props; if (numColumns > 1) { + const {item, index} = info; + invariant(Array.isArray(item), 'Expected array of items with numColumns > 1'); return ( - {item.map((it, kk) => - ) - } + {item.map((it, kk) => { + const element = renderItem({item: it, index: index * numColumns + kk}); + return element && React.cloneElement(element, {key: kk}); + })} ); } else { - return ; + return renderItem(info); } }; @@ -340,7 +350,7 @@ class FlatList extends React.PureComponent, vo return ( , - ItemComponent: ReactClass<{item: Item, index: number}>, - SectionHeaderComponent?: ReactClass<{info: Object}>, - SeparatorComponent?: ReactClass<*>, // not supported yet + renderItem: ({item: Item, index: number}) => ?React.Element<*>, + renderSectionHeader?: ({section: Object}) => ?React.Element<*>, + SeparatorComponent?: ?ReactClass<*>, // not supported yet // Provide either `items` or `sections` items?: ?Array, // By default, an Item is assumed to be {key: string} @@ -163,13 +163,12 @@ class MetroListView extends React.Component { } _renderFooter = () => ; _renderRow = (item, sectionID, rowID, highlightRow) => { - const {ItemComponent} = this.props; - return ; + return this.props.renderItem({item, index: rowID}); }; _renderSectionHeader = (section, sectionID) => { - const {SectionHeaderComponent} = this.props; - invariant(SectionHeaderComponent, 'Must provide SectionHeaderComponent with sections prop'); - return ; + const {renderSectionHeader} = this.props; + invariant(renderSectionHeader, 'Must provide renderSectionHeader with sections prop'); + return renderSectionHeader({section}); } _renderSeparator = (sID, rID) => ; } diff --git a/Libraries/Experimental/SectionList.js b/Libraries/Experimental/SectionList.js index 141b6b38c..65bb78ac9 100644 --- a/Libraries/Experimental/SectionList.js +++ b/Libraries/Experimental/SectionList.js @@ -47,7 +47,7 @@ type SectionBase = { key: string, // Optional props will override list-wide props just for this section. - ItemComponent?: ?ReactClass<{item: SectionItemT, index: number}>, + renderItem?: ?({item: SectionItemT, index: number}) => ?React.Element<*>, SeparatorComponent?: ?ReactClass<*>, keyExtractor?: (item: SectionItemT) => string, @@ -55,10 +55,6 @@ type SectionBase = { // FooterComponent?: ?ReactClass<*>, // HeaderComponent?: ?ReactClass<*>, // onViewableItemsChanged?: ({viewableItems: Array, changed: Array}) => void, - - // TODO: support recursive sections - // SectionHeaderComponent?: ?ReactClass<{section: SectionBase<*>}>, - // sections?: ?Array
; }; type RequiredProps> = { @@ -67,9 +63,9 @@ type RequiredProps> = { type OptionalProps> = { /** - * Default renderer for every item in every section. + * Default renderer for every item in every section. Can be over-ridden on a per-section basis. */ - ItemComponent: ReactClass<{item: Item, index: number}>, + renderItem: ({item: Item, index: number}) => ?React.Element<*>, /** * Rendered in between adjacent Items within each section. */ @@ -85,7 +81,7 @@ type OptionalProps> = { /** * Rendered at the top of each section. Sticky headers are not yet supported. */ - SectionHeaderComponent?: ?ReactClass<{section: SectionT}>, + renderSectionHeader?: ?({section: SectionT}) => ?React.Element<*>, /** * Rendered in between each section. */ @@ -93,7 +89,7 @@ type OptionalProps> = { /** * Warning: Virtualization can drastically improve memory consumption for long lists, but trashes * the state of items when they scroll out of the render window, so make sure all relavent data is - * stored outside of the recursive `ItemComponent` instance tree. + * stored outside of the recursive `renderItem` instance tree. */ enableVirtualization?: ?boolean, keyExtractor: (item: Item, index: number) => string, diff --git a/Libraries/Experimental/VirtualizedList.js b/Libraries/Experimental/VirtualizedList.js index 403072a3e..bf10add14 100644 --- a/Libraries/Experimental/VirtualizedList.js +++ b/Libraries/Experimental/VirtualizedList.js @@ -47,7 +47,7 @@ const {computeWindowedRenderLimits} = require('VirtualizeUtils'); import type {ViewabilityConfig, ViewToken} from 'ViewabilityHelper'; type Item = any; -type ItemComponentType = ReactClass<{item: Item, index: number}>; +type renderItemType = ({item: Item, index: number}) => ?React.Element<*>; /** * Renders a virtual list of items given a data blob and accessor functions. Items that are outside @@ -63,7 +63,7 @@ type ItemComponentType = ReactClass<{item: Item, index: number}>; * */ type RequiredProps = { - ItemComponent: ItemComponentType, + renderItem: renderItemType, /** * The default accessor functions assume this is an Array<{key: string}> but you can override * getItem, getItemCount, and keyExtractor to handle any type of index-based data. @@ -633,7 +633,7 @@ class CellRenderer extends React.Component { onLayout: (event: Object, cellKey: string, index: number) => void, onUnmount: (cellKey: string) => void, parentProps: { - ItemComponent: ItemComponentType, + renderItem: renderItemType, getItemLayout?: ?Function, shouldItemUpdate: ( props: {item: Item, index: number}, @@ -654,8 +654,9 @@ class CellRenderer extends React.Component { } render() { const {item, index, parentProps} = this.props; - const {ItemComponent, getItemLayout} = parentProps; - const element = ; + const {renderItem, getItemLayout} = parentProps; + invariant(renderItem, 'no renderItem!'); + const element = renderItem({item, index}); if (getItemLayout && !parentProps.debug) { return element; } diff --git a/Libraries/Experimental/VirtualizedSectionList.js b/Libraries/Experimental/VirtualizedSectionList.js index aea1ab174..c0bacd3c8 100644 --- a/Libraries/Experimental/VirtualizedSectionList.js +++ b/Libraries/Experimental/VirtualizedSectionList.js @@ -51,7 +51,7 @@ type SectionBase = { key: string, // Optional props will override list-wide props just for this section. - ItemComponent?: ?ReactClass<{item: SectionItem, index: number}>, + renderItem?: ?({item: SectionItem, index: number}) => ?React.Element<*>, SeparatorComponent?: ?ReactClass<*>, keyExtractor?: (item: SectionItem) => string, @@ -59,10 +59,6 @@ type SectionBase = { // FooterComponent?: ?ReactClass<*>, // HeaderComponent?: ?ReactClass<*>, // onViewableItemsChanged?: ({viewableItems: Array, changed: Array}) => void, - - // TODO: support recursive sections - // SectionHeaderComponent?: ?ReactClass<{section: SectionBase}>, - // sections?: ?Array
; }; type RequiredProps = { @@ -73,15 +69,19 @@ type OptionalProps = { /** * Rendered after the last item in the last section. */ - FooterComponent?: ?ReactClass<*>, + ListFooterComponent?: ?ReactClass<*>, + /** + * Rendered at the very beginning of the list. + */ + ListHeaderComponent?: ?ReactClass<*>, /** * Default renderer for every item in every section. */ - ItemComponent: ReactClass<{item: Item, index: number}>, + renderItem: ({item: Item, index: number}) => ?React.Element<*>, /** * Rendered at the top of each section. In the future, a sticky option will be added. */ - SectionHeaderComponent?: ?ReactClass<{section: SectionT}>, + renderSectionHeader?: ?({section: SectionT}) => ?React.Element<*>, /** * Rendered at the bottom of every Section, except the very last one, in place of the normal * SeparatorComponent. @@ -90,11 +90,11 @@ type OptionalProps = { /** * Rendered at the bottom of every Item except the very last one in the last section. */ - SeparatorComponent?: ?ReactClass<*>, + ItemSeparatorComponent?: ?ReactClass<*>, /** * Warning: Virtualization can drastically improve memory consumption for long lists, but trashes * the state of items when they scroll out of the render window, so make sure all relavent data is - * stored outside of the recursive `ItemComponent` instance tree. + * stored outside of the recursive `renderItem` instance tree. */ enableVirtualization?: ?boolean, keyExtractor: (item: Item, index: number) => string, @@ -220,14 +220,16 @@ class VirtualizedSectionList if (!info) { return null; } else if (info.index == null) { - const {SectionHeaderComponent} = this.props; - return SectionHeaderComponent ? : null; + const {renderSectionHeader} = this.props; + return renderSectionHeader ? renderSectionHeader({section: info.section}) : null; } else { - const ItemComponent = info.section.ItemComponent || this.props.ItemComponent; + const renderItem = info.section.renderItem || + this.props.renderItem; const SeparatorComponent = this._getSeparatorComponent(index, info); + invariant(renderItem, 'no renderItem!'); return ( - + {renderItem({item, index: info.index || 0})} {SeparatorComponent && } ); @@ -239,7 +241,7 @@ class VirtualizedSectionList if (!info) { return null; } - const SeparatorComponent = info.section.SeparatorComponent || this.props.SeparatorComponent; + const SeparatorComponent = info.section.SeparatorComponent || this.props.ItemSeparatorComponent; const {SectionSeparatorComponent} = this.props; const isLastItemInList = index === this.state.childProps.getItemCount() - 1; const isLastItemInSection = info.index === info.section.data.length - 1; @@ -265,8 +267,10 @@ class VirtualizedSectionList return { childProps: { ...props, - ItemComponent: this._renderItem, - SeparatorComponent: undefined, // Rendered with ItemComponent + FooterComponent: this.props.ListFooterComponent, + HeaderComponent: this.props.ListHeaderComponent, + renderItem: this._renderItem, + SeparatorComponent: undefined, // Rendered with renderItem data: props.sections, getItemCount: () => itemCount, getItem, diff --git a/Libraries/Experimental/__flowtests__/FlatList-flowtest.js b/Libraries/Experimental/__flowtests__/FlatList-flowtest.js index 83d90947b..13fec8472 100644 --- a/Libraries/Experimental/__flowtests__/FlatList-flowtest.js +++ b/Libraries/Experimental/__flowtests__/FlatList-flowtest.js @@ -14,64 +14,77 @@ const FlatList = require('FlatList'); const React = require('react'); -class MyListItem extends React.Component { - props: { - item: { - title: string, - }, - }; - render() { - return ; - } +function renderMyListItem(info: {item: {title: string}, index: number}) { + return ; } module.exports = { - testBadDataWithTypicalItemComponent(): React.Element<*> { + testEverythingIsFine() { + const data = [{ + title: 'Title Text', + key: 1, + }]; + return ; + }, + + testBadDataWithTypicalItem() { // $FlowExpectedError - bad title type 6, should be string const data = [{ title: 6, key: 1, }]; - return ; + return ; }, - testMissingFieldWithTypicalItemComponent(): React.Element<*> { + testMissingFieldWithTypicalItem() { const data = [{ key: 1, }]; // $FlowExpectedError - missing title - return ; + return ; }, - testGoodDataWithGoodCustomItemComponentFunction() { + testGoodDataWithBadCustomRenderItemFunction() { const data = [{ - widgetCount: 3, + widget: 6, key: 1, }]; return ( => - + renderItem={(info) => + // $FlowExpectedError - bad widgetCount type 6, should be Object + {info.item.widget.missingProp} } data={data} /> ); }, - testBadNonInheritedDefaultProp(): React.Element<*> { - const data = []; - // $FlowExpectedError - bad numColumns type "lots" - return ; + testBadRenderItemFunction() { + const data = [{ + title: 'foo', + key: 1, + }]; + return [ + // $FlowExpectedError - title should be inside `item` + } data={data} />, + // $FlowExpectedError - bad index type string, should be number + } data={data} />, + // $FlowExpectedError - bad title type number, should be string + } data={data} />, + // EverythingIsFine + } data={data} />, + ]; }, - testBadInheritedDefaultProp(): React.Element<*> { - const data = []; - // $FlowExpectedError - bad windowSize type "big" - return ; - }, - - testMissingData(): React.Element<*> { - // $FlowExpectedError - missing `data` prop - return ; + testOtherBadProps() { + return [ + // $FlowExpectedError - bad numColumns type "lots" + , + // $FlowExpectedError - bad windowSize type "big" + , + // $FlowExpectedError - missing `data` prop + , + ]; }, }; diff --git a/Libraries/Experimental/__flowtests__/SectionList-flowtest.js b/Libraries/Experimental/__flowtests__/SectionList-flowtest.js index fda0e5c45..dc1941f2d 100644 --- a/Libraries/Experimental/__flowtests__/SectionList-flowtest.js +++ b/Libraries/Experimental/__flowtests__/SectionList-flowtest.js @@ -14,55 +14,49 @@ const React = require('react'); const SectionList = require('SectionList'); -class MyListItem extends React.Component { - props: { - item: { - title: string, - }, - }; - render() { - return ; - } +function renderMyListItem(info: {item: {title: string}, index: number}) { + return ; } -class MyHeader extends React.Component { - props: { - section: { - fooNumber: number, - } - }; - render() { - return ; - } -} +const renderMyHeader = ({section}: {section: {fooNumber: number} & Object}) => ; module.exports = { - testGoodDataWithGoodCustomItemComponentFunction() { + testGoodDataWithGoodItem() { const sections = [{ key: 'a', data: [{ - widgetCount: 3, + title: 'foo', key: 1, }], }]; - return ( - => - - } - sections={sections} - /> - ); + return ; + }, + + testBadRenderItemFunction() { + const sections = [{ + key: 'a', data: [{ + title: 'foo', + key: 1, + }], + }]; + return [ + // $FlowExpectedError - title should be inside `item` + } sections={sections} />, + // $FlowExpectedError - bad index type string, should be number + } sections={sections} />, + // EverythingIsFine + } sections={sections} />, + ]; }, testBadInheritedDefaultProp(): React.Element<*> { const sections = []; // $FlowExpectedError - bad windowSize type "big" - return ; + return ; }, testMissingData(): React.Element<*> { // $FlowExpectedError - missing `sections` prop - return ; + return ; }, testBadSectionsShape(): React.Element<*> { @@ -73,7 +67,7 @@ module.exports = { }], }]; // $FlowExpectedError - section missing `data` field - return ; + return ; }, testBadSectionsMetadata(): React.Element<*> { @@ -86,8 +80,8 @@ module.exports = { }]; return ( );