From 57a1e7c000e0b035cdf5ac7cb256ded59ac2fcfb Mon Sep 17 00:00:00 2001 From: Alan Kenyon Date: Mon, 20 May 2019 07:42:49 -0700 Subject: [PATCH] VirtualizedList.RenderItem throws when using function component with hooks (#24832) Summary: `` will throw an error if the `renderItem` Prop component uses hooks. Function components without hooks and class components work without issue. Super contrived Example ```{js} function FlatListItem({ item }) { React.useEffect(() => console.log(item),[]) return ({item}); } ``` Example Error: ``` Invariant Violation: Hooks can only be called inside the body of a function component. (https://fb.me/react-invalid-hook-call) This error is located at: in CellRenderer (at VirtualizedList.js:688) in RCTScrollContentView (at ScrollView.js:976) in RCTScrollView (at ScrollView.js:1115) in ScrollView (at VirtualizedList.js:1081) in VirtualizedList (at FlatList.js:632) in FlatList (at WithoutScrollbars.js:21) ... ``` ## Changelog [General] [Added] - VirtualizedList ListItemComponent. An alternative to renderItem that accepts function components with hooks. [General][Added] - FlatList ListItemComponent. An alternative to renderItem that accepts function components with hooks. [General][Added] - VirtualizedList and FlatList tests and updated RNTester example Pull Request resolved: https://github.com/facebook/react-native/pull/24832 Reviewed By: sahrens Differential Revision: D15334020 Pulled By: cpojer fbshipit-source-id: 882db722fd6e22f07260b08091b3456d1c66c2c8 --- Libraries/Inspector/NetworkOverlay.js | 2 +- Libraries/Lists/FlatList.js | 143 +++++++---- Libraries/Lists/VirtualizedList.js | 72 +++++- Libraries/Lists/__tests__/FlatList-test.js | 35 +++ .../Lists/__tests__/VirtualizedList-test.js | 55 +++++ .../__snapshots__/FlatList-test.js.snap | 223 ++++++++++++++++++ .../VirtualizedList-test.js.snap | 108 +++++++++ RNTester/js/FlatListExample.js | 37 ++- 8 files changed, 602 insertions(+), 73 deletions(-) diff --git a/Libraries/Inspector/NetworkOverlay.js b/Libraries/Inspector/NetworkOverlay.js index 4e93151c13a..a39370bcee8 100644 --- a/Libraries/Inspector/NetworkOverlay.js +++ b/Libraries/Inspector/NetworkOverlay.js @@ -325,7 +325,7 @@ class NetworkOverlay extends React.Component { WebSocketInterceptor.disableInterception(); } - _renderItem = ({item, index}): ?React.Element => { + _renderItem = ({item, index}): React.Element => { const tableRowViewStyle = [ styles.tableRow, index % 2 === 1 ? styles.tableRowOdd : styles.tableRowEven, diff --git a/Libraries/Lists/FlatList.js b/Libraries/Lists/FlatList.js index 5d17580c163..292c6e1cc63 100644 --- a/Libraries/Lists/FlatList.js +++ b/Libraries/Lists/FlatList.js @@ -24,15 +24,20 @@ import type { ViewToken, ViewabilityConfigCallbackPair, } from './ViewabilityHelper'; -import type {Props as VirtualizedListProps} from './VirtualizedList'; - -export type SeparatorsObj = { - highlight: () => void, - unhighlight: () => void, - updateProps: (select: 'leading' | 'trailing', newProps: Object) => void, -}; +import type { + Props as VirtualizedListProps, + RenderItemType, + RenderItemProps, +} from './VirtualizedList'; type RequiredProps = { + /** + * For simplicity, data is just a plain array. If you want to use something else, like an + * immutable list, use the underlying `VirtualizedList` directly. + */ + data: ?$ReadOnlyArray, +}; +type OptionalProps = { /** * Takes an item from `data` and renders it into the list. Example usage: * @@ -59,18 +64,7 @@ type RequiredProps = { * `highlight` and `unhighlight` (which set the `highlighted: boolean` prop) are insufficient for * your use-case. */ - renderItem: (info: { - item: ItemT, - index: number, - separators: SeparatorsObj, - }) => ?React.Node, - /** - * For simplicity, data is just a plain array. If you want to use something else, like an - * immutable list, use the underlying `VirtualizedList` directly. - */ - data: ?$ReadOnlyArray, -}; -type OptionalProps = { + renderItem?: ?RenderItemType, /** * Rendered in between each item, but not at the top or bottom. By default, `highlighted` and * `leadingItem` props are provided. `renderItem` provides `separators.highlight`/`unhighlight` @@ -78,6 +72,33 @@ type OptionalProps = { * `separators.updateProps`. */ ItemSeparatorComponent?: ?React.ComponentType, + /** + * Takes an item from `data` and renders it into the list. Example usage: + * + * ( + * + * )} + * data={[{title: 'Title Text', key: 'item1'}]} + * ListItemComponent={({item, separators}) => ( + * this._onPress(item)} + * onShowUnderlay={separators.highlight} + * onHideUnderlay={separators.unhighlight}> + * + * {item.title} + * + * + * )} + * /> + * + * Provides additional metadata like `index` if you need it, as well as a more generic + * `separators.updateProps` function which let's you set whatever props you want to change the + * rendering of either the leading separator or trailing separator in case the more common + * `highlight` and `unhighlight` (which set the `highlighted: boolean` prop) are insufficient for + * your use-case. + */ + ListItemComponent?: ?React.ComponentType, /** * Rendered when the list is empty. Can be a React Component Class, a render function, or * a rendered element. @@ -598,47 +619,71 @@ class FlatList extends React.PureComponent, void> { }; } - _renderItem = (info: Object): ?React.Node => { - 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) => { - const element = renderItem({ - item: it, - index: index * numColumns + kk, - separators: info.separators, - }); - return element != null ? ( - {element} - ) : null; - })} - - ); - } else { - return renderItem(info); - } + _renderer = () => { + const { + ListItemComponent, + renderItem, + numColumns, + columnWrapperStyle, + } = this.props; + + let virtualizedListRenderKey = ListItemComponent + ? 'ListItemComponent' + : 'renderItem'; + + const renderer = props => { + if (ListItemComponent) { + return ; + } else if (renderItem) { + return renderItem(props); + } else { + return null; + } + }; + + return { + [virtualizedListRenderKey]: (info: RenderItemProps) => { + if (numColumns > 1) { + const {item, index} = info; + invariant( + Array.isArray(item), + 'Expected array of items with numColumns > 1', + ); + return ( + + {item.map((it, kk) => { + const element = renderer({ + item: it, + index: index * numColumns + kk, + separators: info.separators, + }); + return element != null ? ( + {element} + ) : null; + })} + + ); + } else { + return renderer(info); + } + }, + }; }; render() { return ( ); } diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index b896b3b5fb7..0281d87c8a3 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -17,7 +17,6 @@ const ReactNative = require('../Renderer/shims/ReactNative'); const RefreshControl = require('../Components/RefreshControl/RefreshControl'); const ScrollView = require('../Components/ScrollView/ScrollView'); const StyleSheet = require('../StyleSheet/StyleSheet'); -const UIManager = require('../ReactNative/UIManager'); const View = require('../Components/View/View'); const ViewabilityHelper = require('./ViewabilityHelper'); @@ -37,7 +36,21 @@ import type { type Item = any; -export type renderItemType = (info: any) => ?React.Element; +export type Separators = { + highlight: () => void, + unhighlight: () => void, + updateProps: (select: 'leading' | 'trailing', newProps: Object) => void, +}; + +export type RenderItemProps = { + item: ItemT, + index: number, + separators: Separators, +}; + +export type RenderItemType = ( + info: RenderItemProps, +) => React.Node; type ViewabilityHelperCallbackTuple = { viewabilityHelper: ViewabilityHelper, @@ -48,9 +61,6 @@ type ViewabilityHelperCallbackTuple = { }; type RequiredProps = { - // TODO: Conflicts with the optional `renderItem` in - // `VirtualizedSectionList`'s props. - renderItem: $FlowFixMe, /** * The default accessor functions assume this is an Array<{key: string} | {id: string}> but you can override * getItem, getItemCount, and keyExtractor to handle any type of index-based data. @@ -66,6 +76,9 @@ type RequiredProps = { getItemCount: (data: any) => number, }; type OptionalProps = { + // TODO: Conflicts with the optional `renderItem` in + // `VirtualizedSectionList`'s props. + renderItem?: $FlowFixMe>, /** * `debug` will turn on extra logging and visual overlays to aid with debugging both usage and * implementation, but with a significant perf hit. @@ -111,6 +124,11 @@ type OptionalProps = { * or a render function. Defaults to using View. */ CellRendererComponent?: ?React.ComponentType, + /** + * Each data item is rendered using this element. Can be a React Component Class, + * or a render function. + */ + ListItemComponent?: ?React.ComponentType, /** * Rendered when the list is empty. Can be a React Component Class, a render function, or * a rendered element. @@ -1664,7 +1682,8 @@ class CellRenderer extends React.Component< onUpdateSeparators: (cellKeys: Array, props: Object) => void, parentProps: { getItemLayout?: ?Function, - renderItem: renderItemType, + renderItem?: ?RenderItemType, + ListItemComponent?: ?(React.ComponentType | React.Element), }, prevCellKey: ?string, }, @@ -1725,6 +1744,36 @@ class CellRenderer extends React.Component< this.props.onUnmount(this.props.cellKey); } + _renderElement(renderItem, ListItemComponent, item, index) { + if (renderItem && ListItemComponent) { + console.warn( + 'VirtualizedList: Both ListItemComponent and renderItem props are present. ListItemComponent will take' + + ' precedence over renderItem.', + ); + } + + if (ListItemComponent) { + return React.createElement(ListItemComponent, { + item, + index, + separators: this._separators, + }); + } + + if (renderItem) { + return renderItem({ + item, + index, + separators: this._separators, + }); + } + + invariant( + false, + 'VirtualizedList: Either ListItemComponent or renderItem props are required but none were found.', + ); + } + render() { const { CellRendererComponent, @@ -1736,13 +1785,14 @@ class CellRenderer extends React.Component< inversionStyle, parentProps, } = this.props; - const {renderItem, getItemLayout} = parentProps; - invariant(renderItem, 'no renderItem!'); - const element = renderItem({ + const {renderItem, getItemLayout, ListItemComponent} = parentProps; + const element = this._renderElement( + renderItem, + ListItemComponent, item, index, - separators: this._separators, - }); + ); + 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 diff --git a/Libraries/Lists/__tests__/FlatList-test.js b/Libraries/Lists/__tests__/FlatList-test.js index e1c605aae11..1df7e484c76 100644 --- a/Libraries/Lists/__tests__/FlatList-test.js +++ b/Libraries/Lists/__tests__/FlatList-test.js @@ -25,6 +25,41 @@ describe('FlatList', () => { ); expect(component).toMatchSnapshot(); }); + it('renders simple list (multiple columns)', () => { + const component = ReactTestRenderer.create( + } + numColumns={2} + />, + ); + expect(component).toMatchSnapshot(); + }); + it('renders simple list using ListItemComponent', () => { + function ListItemComponent({item}) { + return ; + } + const component = ReactTestRenderer.create( + , + ); + expect(component).toMatchSnapshot(); + }); + it('renders simple list using ListItemComponent (multiple columns)', () => { + function ListItemComponent({item}) { + return ; + } + const component = ReactTestRenderer.create( + , + ); + expect(component).toMatchSnapshot(); + }); it('renders empty list', () => { const component = ReactTestRenderer.create( } />, diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js index d421d1b9a78..c4873374e0e 100644 --- a/Libraries/Lists/__tests__/VirtualizedList-test.js +++ b/Libraries/Lists/__tests__/VirtualizedList-test.js @@ -28,6 +28,61 @@ describe('VirtualizedList', () => { expect(component).toMatchSnapshot(); }); + it('renders simple list using ListItemComponent', () => { + function ListItemComponent({item}) { + return ; + } + const component = ReactTestRenderer.create( + data[index]} + getItemCount={data => data.length} + />, + ); + expect(component).toMatchSnapshot(); + }); + + it('warns if both renderItem or ListItemComponent are specified. Uses ListItemComponent', () => { + jest.spyOn(global.console, 'warn'); + function ListItemComponent({item}) { + return ; + } + const component = ReactTestRenderer.create( + ( + + )} + getItem={(data, index) => data[index]} + getItemCount={data => data.length} + />, + ); + + expect(console.warn.mock.calls).toEqual([ + [ + 'VirtualizedList: Both ListItemComponent and renderItem props are present. ListItemComponent will take precedence over renderItem.', + ], + ]); + expect(component).toMatchSnapshot(); + console.warn.mockRestore(); + }); + + it('throws if no renderItem or ListItemComponent', () => { + const componentFactory = () => + ReactTestRenderer.create( + data[index]} + getItemCount={data => data.length} + />, + ); + expect(componentFactory).toThrow( + 'VirtualizedList: Either ListItemComponent or renderItem props are required but none were found.', + ); + }); + it('renders empty list', () => { const component = ReactTestRenderer.create( `; +exports[`FlatList renders simple list (multiple columns) 1`] = ` + + + + + + + + + + + + + + + +`; + exports[`FlatList renders simple list 1`] = ` `; + +exports[`FlatList renders simple list using ListItemComponent (multiple columns) 1`] = ` + + + + + + + + + + + + + + + +`; + +exports[`FlatList renders simple list using ListItemComponent 1`] = ` + + + + + + + + + + + + + +`; diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap index 2ae233201f7..86faf0fa8aa 100644 --- a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap +++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap @@ -811,6 +811,70 @@ exports[`VirtualizedList renders simple list 1`] = ` `; +exports[`VirtualizedList renders simple list using ListItemComponent 1`] = ` + + + + + + + + + + + + + +`; + exports[`VirtualizedList test getItem functionality where data is not an Array 1`] = ` `; + +exports[`VirtualizedList warns if both renderItem or ListItemComponent are specified. Uses ListItemComponent 1`] = ` + + + + + + + +`; diff --git a/RNTester/js/FlatListExample.js b/RNTester/js/FlatListExample.js index f8587040584..733f0737f0f 100644 --- a/RNTester/js/FlatListExample.js +++ b/RNTester/js/FlatListExample.js @@ -51,6 +51,7 @@ type State = {| logViewable: boolean, virtualized: boolean, empty: boolean, + useFlatListItemComponent: boolean, |}; class FlatListExample extends React.PureComponent { @@ -64,6 +65,7 @@ class FlatListExample extends React.PureComponent { logViewable: false, virtualized: true, empty: false, + useFlatListItemComponent: false, }; _onChangeFilterText = filterText => { @@ -95,6 +97,7 @@ class FlatListExample extends React.PureComponent { const filter = item => filterRegex.test(item.text) || filterRegex.test(item.title); const filteredData = this.state.data.filter(filter); + const flatListItemRendererProps = this._renderItemComponent(); return ( @@ -118,6 +121,7 @@ class FlatListExample extends React.PureComponent { {renderSmallSwitchOption(this, 'inverted')} {renderSmallSwitchOption(this, 'empty')} {renderSmallSwitchOption(this, 'debug')} + {renderSmallSwitchOption(this, 'useFlatListItemComponent')} @@ -150,9 +154,9 @@ class FlatListExample extends React.PureComponent { onViewableItemsChanged={this._onViewableItemsChanged} ref={this._captureRef} refreshing={false} - renderItem={this._renderItemComponent} contentContainerStyle={styles.list} viewabilityConfig={VIEWABILITY_CONFIG} + {...flatListItemRendererProps} /> @@ -173,17 +177,26 @@ class FlatListExample extends React.PureComponent { })); }; _onRefresh = () => Alert.alert('onRefresh: nothing to refresh :P'); - _renderItemComponent = ({item, separators}) => { - return ( - - ); + _renderItemComponent = () => { + const flatListPropKey = this.state.useFlatListItemComponent + ? 'ListItemComponent' + : 'renderItem'; + + return { + renderItem: undefined, + [flatListPropKey]: ({item, separators}) => { + return ( + + ); + }, + }; }; // This is called when items change viewability by scrolling into or out of // the viewable area.