From 6c5088f763099d744b83028611edb4bebebdb172 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Feb 2023 16:19:14 -0800 Subject: [PATCH] Make FlatList permissive of ArrayLike data (#36236) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36236 D38198351 (https://github.com/facebook/react-native/commit/d574ea3526e713eae2c6e20c7a68fa66ff4ad7d2) addedd a guard to FlatList, to no-op if passed `data` that was not an array. This broke functionality where Realm had documented using `Realm.Results` with FlatList. `Real.Results` is an array-like JSI object, but not actually an array, and fails any `Array.isArray()` checks. This change loosens the FlatList contract, to explicitly allow array-like non-array entities. The requirement align to Flow `ArrayLike`, which allows both arrays, and objects which provide a length and indexer. Flow `$ArrayLike` currently also requires an iterator, but this is seemingly a mistake in the type definition, and not enforced. Though `Realm.Results` has all the methods of TS `ReadonlyArray`, RN has generally assumes its array inputs will pass `Array.isArray()`. This includes any array props still being checked [via prop-types](https://github.com/facebook/prop-types/blob/044efd7a108556c7660f6b62092756666e39d74b/factoryWithTypeCheckers.js#L548). This change intentionally does not yet change the parameter type of `getItemLayout()`, which is already too loose (allowing mutable arrays). Changing this is a breaking change, that would be disruptive to backport, so we separate it into a different commit that will be landed as part of 0.72 (see next diff in the stack). Changelog: [General][Changed] - Make FlatList permissive of ArrayLike data Reviewed By: yungsters Differential Revision: D43465654 fbshipit-source-id: 3ed8c76c15da680560d7639b7cc43272f3e46ac3 --- Libraries/Lists/FlatList.d.ts | 12 +- Libraries/Lists/FlatList.js | 27 ++- Libraries/Lists/__tests__/FlatList-test.js | 25 +++ .../__snapshots__/FlatList-test.js.snap | 157 ++++++++++++++++++ 4 files changed, 208 insertions(+), 13 deletions(-) diff --git a/Libraries/Lists/FlatList.d.ts b/Libraries/Lists/FlatList.d.ts index 3d9cf32840eb9f..4f60e18c4daa28 100644 --- a/Libraries/Lists/FlatList.d.ts +++ b/Libraries/Lists/FlatList.d.ts @@ -14,9 +14,9 @@ import type { VirtualizedListProps, } from './VirtualizedList'; 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 extends VirtualizedListProps { /** @@ -40,10 +40,10 @@ export interface FlatListProps extends VirtualizedListProps { | 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 | null | undefined; + data: ArrayLike | null | undefined; /** * A marker property for telling the list to re-render (since it implements PureComponent). diff --git a/Libraries/Lists/FlatList.js b/Libraries/Lists/FlatList.js index 56748eaf75240d..9b280f237cfbc4 100644 --- a/Libraries/Lists/FlatList.js +++ b/Libraries/Lists/FlatList.js @@ -30,10 +30,10 @@ const React = require('react'); 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. + * An array (or array-like list) of items to render. Other data types can be + * used by targetting VirtualizedList directly. */ - data: ?$ReadOnlyArray, + data: ?$ArrayLike, |}; type OptionalProps = {| /** @@ -163,6 +163,11 @@ function numColumnsOrDefault(numColumns: ?number) { return numColumns ?? 1; } +function isArrayLike(data: mixed): boolean { + // $FlowExpectedError[incompatible-use] + return typeof Object(data).length === 'number'; +} + type FlatListProps = {| ...RequiredProps, ...OptionalProps, @@ -497,8 +502,10 @@ class FlatList extends React.PureComponent, void> { ); } - // $FlowFixMe[missing-local-annot] - _getItem = (data: Array, index: number) => { + _getItem = ( + data: $ArrayLike, + index: number, + ): ?(ItemT | $ReadOnlyArray) => { const numColumns = numColumnsOrDefault(this.props.numColumns); if (numColumns > 1) { const ret = []; @@ -515,8 +522,14 @@ class FlatList extends React.PureComponent, void> { } }; - _getItemCount = (data: ?Array): number => { - if (Array.isArray(data)) { + _getItemCount = (data: ?$ArrayLike): number => { + // Legacy behavior of FlatList was to forward "undefined" length if invalid + // data like a non-arraylike object is passed. VirtualizedList would then + // coerce this, and the math would work out to no-op. For compatibility, if + // invalid data is passed, we tell VirtualizedList there are zero items + // available to prevent it from trying to read from the invalid data + // (without propagating invalidly typed data). + if (data != null && isArrayLike(data)) { const numColumns = numColumnsOrDefault(this.props.numColumns); return numColumns > 1 ? Math.ceil(data.length / numColumns) : data.length; } else { diff --git a/Libraries/Lists/__tests__/FlatList-test.js b/Libraries/Lists/__tests__/FlatList-test.js index bc7c9d1faf16cc..e9f9197b2c5038 100644 --- a/Libraries/Lists/__tests__/FlatList-test.js +++ b/Libraries/Lists/__tests__/FlatList-test.js @@ -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( + } + />, + ); + expect(component).toMatchSnapshot(); + }); + it('ignores invalid data', () => { + const component = ReactTestRenderer.create( + } + />, + ); + expect(component).toMatchSnapshot(); + }); }); diff --git a/Libraries/Lists/__tests__/__snapshots__/FlatList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/FlatList-test.js.snap index 03e7c9533385bb..2fec3a235fb118 100644 --- a/Libraries/Lists/__tests__/__snapshots__/FlatList-test.js.snap +++ b/Libraries/Lists/__tests__/__snapshots__/FlatList-test.js.snap @@ -1,5 +1,63 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`FlatList ignores invalid data 1`] = ` + + + +`; + exports[`FlatList renders all the bells and whistles 1`] = ` `; +exports[`FlatList renders array-like data 1`] = ` + + + + + + + + + + + + + +`; + exports[`FlatList renders empty list 1`] = `