VirtualizedList.RenderItem throws when using function component with hooks

[adkenyon]

Summary

<VirtualizedList /> will throw an error if the renderItem Prop component uses hooks. Function components without hooks and class components work without issue.

Super contrived Example

function FlatListItem({ item }) {
  React.useEffect(() => console.log(item),[])

 return (<Text>{item}</Text>);
}

<FlatList data={[1, 2, 3]} renderItem={FlatListItem} />

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

Test Plan

Updated JS tests for VirtualizedList and tested on local build.

[cpojer]

Nice! Thank you for fixing this issue.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[cpojer]

I spent some time thinking about this and I'm worried about the performance implications of this breaking change because of the added wrapping for each cell. I think renderItem is also a misleading prop name.

How about this:

• Add a new prop called item which takes a React component
• If item is specified, use it with React.createElement
• If item isn't specified, fall back to renderItem (this ensures the default behavior stays the same)
• If both item and renderItem are specified, we should console.warn and explain that one takes precedence over the other.

Then in the future we can remove renderItem completely.

[analysis-bot]

Code analysis results:

• flow found some issues.

[analysis-bot]

Code analysis results:

• eslint found some issues. Run yarn lint --fix to automatically fix problems.

[adkenyon]

@cpojer Implemented your suggestions.

• I Used ListItemComponent rather than Item. I think it follows the existing naming convention.
• Minor flow type cleanup
• After adding console.warn for the condition that both ListItemComponent and renderItem are present I noticed that FlatList would constantly throw warnings as FlatList always returns an implementation of renderItem (to handle multiple columns). I was able to fix this for FlatList, but VirtualizedSectionList and SectionList have the same issue. I'm thinking that I'll follow up with the ListItemComponent implementation for VirtualizedSectionList and SectionList on another PR.

[facebook-github-bot]

@cpojer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[sahrens]

Awesome, much cleaner :)

[cpojer]

I applied the change internally and I'm trying to land it :) Thanks for going through all the iterations with us @adkenyon!

[react-native-bot]

This pull request was successfully merged by @adkenyon in 57a1e7c.

^{When will my fix make it into a release? | Upcoming Releases}