Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[1/2] Make FlatList permissive of ArrayLike data #36236

Closed
wants to merge 1 commit into from
Closed

[1/2] Make FlatList permissive of ArrayLike data #36236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions Libraries/Lists/FlatList.d.ts
View file
Open in desktop
Expand Up @@ -14,9 +14,9 @@ import type {
VirtualizedListProps,
} from '@react-native/virtualized-lists';
import type {ScrollViewComponent} from '../Components/ScrollView/ScrollView';
import {StyleProp} from '../StyleSheet/StyleSheet';
import {ViewStyle} from '../StyleSheet/StyleSheetTypes';
import {View} from '../Components/View/View';
import type {StyleProp} from '../StyleSheet/StyleSheet';
import type {ViewStyle} from '../StyleSheet/StyleSheetTypes';
import type {View} from '../Components/View/View';

export interface FlatListProps<ItemT> extends VirtualizedListProps<ItemT> {
/**
Expand All @@ -40,10 +40,10 @@ export interface FlatListProps<ItemT> extends VirtualizedListProps<ItemT> {
| undefined;

/**
* For simplicity, data is just a plain array. If you want to use something else,
* like an immutable list, use the underlying VirtualizedList directly.
* An array (or array-like list) of items to render. Other data types can be
* used by targetting VirtualizedList directly.
*/
data: ReadonlyArray<ItemT> | null | undefined;
data: ArrayLike<ItemT> | null | undefined;

/**
* A marker property for telling the list to re-render (since it implements PureComponent).
Expand Down
16 changes: 9 additions & 7 deletions Libraries/Lists/FlatList.js
View file
Open in desktop
Expand Up @@ -33,10 +33,10 @@ const React = require('react');

type RequiredProps<ItemT> = {|
/**
* For simplicity, data is just a plain array. If you want to use something else, like an
* immutable list, use the underlying `VirtualizedList` directly.
* An array (or array-like list) of items to render. Other data types can be
* used by targetting VirtualizedList directly.
*/
data: ?$ReadOnlyArray<ItemT>,
data: ?$ArrayLike<ItemT>,
|};
type OptionalProps<ItemT> = {|
/**
Expand Down Expand Up @@ -500,8 +500,10 @@ class FlatList<ItemT> extends React.PureComponent<Props<ItemT>, void> {
);
}

// $FlowFixMe[missing-local-annot]
_getItem = (data: Array<ItemT>, index: number) => {
_getItem = (
data: $ArrayLike<ItemT>,
index: number,
): ?(ItemT | $ReadOnlyArray<ItemT>) => {
const numColumns = numColumnsOrDefault(this.props.numColumns);
if (numColumns > 1) {
const ret = [];
Expand All @@ -518,8 +520,8 @@ class FlatList<ItemT> extends React.PureComponent<Props<ItemT>, void> {
}
};

_getItemCount = (data: ?Array<ItemT>): number => {
if (Array.isArray(data)) {
_getItemCount = (data: ?$ArrayLike<ItemT>): number => {
if (data != null && typeof Object(data).length === 'number') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's ArrayLike obj and not null, it must have a length prop defined. ?
And we don't want those with only size defined or do we ?
any specific reason for new Object instance overhead?

Suggested change
if (data != null && typeof Object(data).length === 'number') {
if (data != null && typeof data.length === 'number') {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per MDN this should return the existing object if it is one, but the check would cause an exception if someone passed say, a number, and the point here has been to guard against invalid data gracefully for legacy reasons.

const numColumns = numColumnsOrDefault(this.props.numColumns);
return numColumns > 1 ? Math.ceil(data.length / numColumns) : data.length;
} else {
Expand Down
25 changes: 25 additions & 0 deletions Libraries/Lists/__tests__/FlatList-test.js
View file
Open in desktop
Expand Up @@ -182,4 +182,29 @@ describe('FlatList', () => {

expect(renderItemInThreeColumns).toHaveBeenCalledTimes(7);
});
it('renders array-like data', () => {
const arrayLike = {
length: 3,
0: {key: 'i1'},
1: {key: 'i2'},
2: {key: 'i3'},
};

const component = ReactTestRenderer.create(
<FlatList
data={arrayLike}
renderItem={({item}) => <item value={item.key} />}
/>,
);
expect(component).toMatchSnapshot();
});
it('ignores invalid data', () => {
const component = ReactTestRenderer.create(
<FlatList
data={123456}
renderItem={({item}) => <item value={item.key} />}
/>,
);
expect(component).toMatchSnapshot();
});
});
157 changes: 157 additions & 0 deletions Libraries/Lists/__tests__/__snapshots__/FlatList-test.js.snap
View file
Open in desktop
@@ -1,5 +1,63 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`FlatList ignores invalid data 1`] = `
<RCTScrollView
alwaysBounceVertical={true}
data={123456}
getItem={[Function]}
getItemCount={[Function]}
keyExtractor={[Function]}
onContentSizeChange={null}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onResponderGrant={[Function]}
onResponderReject={[Function]}
onResponderRelease={[Function]}
onResponderTerminationRequest={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
onScrollShouldSetResponder={[Function]}
onStartShouldSetResponder={[Function]}
onStartShouldSetResponderCapture={[Function]}
onTouchCancel={[Function]}
onTouchEnd={[Function]}
onTouchMove={[Function]}
onTouchStart={[Function]}
pagingEnabled={false}
removeClippedSubviews={false}
renderItem={[Function]}
scrollEventThrottle={50}
scrollViewRef={[Function]}
sendMomentumEvents={true}
snapToEnd={true}
snapToStart={true}
stickyHeaderIndices={Array []}
style={
Object {
"flexDirection": "column",
"flexGrow": 1,
"flexShrink": 1,
"overflow": "scroll",
}
}
viewabilityConfigCallbackPairs={Array []}
>
<RCTScrollContentView
collapsable={false}
onLayout={[Function]}
removeClippedSubviews={false}
style={
Array [
false,
undefined,
]
}
/>
</RCTScrollView>
`;

exports[`FlatList renders all the bells and whistles 1`] = `
<RCTScrollView
ItemSeparatorComponent={[Function]}
Expand Down Expand Up @@ -122,6 +180,105 @@ exports[`FlatList renders all the bells and whistles 1`] = `
</RCTScrollView>
`;

exports[`FlatList renders array-like data 1`] = `
<RCTScrollView
alwaysBounceVertical={true}
data={
Object {
"0": Object {
"key": "i1",
},
"1": Object {
"key": "i2",
},
"2": Object {
"key": "i3",
},
"length": 3,
}
}
getItem={[Function]}
getItemCount={[Function]}
keyExtractor={[Function]}
onContentSizeChange={null}
onLayout={[Function]}
onMomentumScrollBegin={[Function]}
onMomentumScrollEnd={[Function]}
onResponderGrant={[Function]}
onResponderReject={[Function]}
onResponderRelease={[Function]}
onResponderTerminationRequest={[Function]}
onScroll={[Function]}
onScrollBeginDrag={[Function]}
onScrollEndDrag={[Function]}
onScrollShouldSetResponder={[Function]}
onStartShouldSetResponder={[Function]}
onStartShouldSetResponderCapture={[Function]}
onTouchCancel={[Function]}
onTouchEnd={[Function]}
onTouchMove={[Function]}
onTouchStart={[Function]}
pagingEnabled={false}
removeClippedSubviews={false}
renderItem={[Function]}
scrollEventThrottle={50}
scrollViewRef={[Function]}
sendMomentumEvents={true}
snapToEnd={true}
snapToStart={true}
stickyHeaderIndices={Array []}
style={
Object {
"flexDirection": "column",
"flexGrow": 1,
"flexShrink": 1,
"overflow": "scroll",
}
}
viewabilityConfigCallbackPairs={Array []}
>
<RCTScrollContentView
collapsable={false}
onLayout={[Function]}
removeClippedSubviews={false}
style={
Array [
false,
undefined,
]
}
>
<View
onFocusCapture={[Function]}
onLayout={[Function]}
style={null}
>
<item
value="i1"
/>
</View>
<View
onFocusCapture={[Function]}
onLayout={[Function]}
style={null}
>
<item
value="i2"
/>
</View>
<View
onFocusCapture={[Function]}
onLayout={[Function]}
style={null}
>
<item
value="i3"
/>
</View>
</RCTScrollContentView>
</RCTScrollView>
`;

exports[`FlatList renders empty list 1`] = `
<RCTScrollView
data={Array []}
Expand Down