Skip to content

Commit 5475f29

Browse files
Luna Weifacebook-github-bot
authored andcommitted
Reland stack for removing defaultProps from VL and remove breaking FlowFixMe
Summary: Changelog: [Internal][Changed] - Remove keyExtractor from defaultProps as part of larger effort to remove defaultProps from VirtualizedList [General][Changed] - Remove data as a prop for VirtualizedSectionList. In the reactnative.dev docs it mentions that sections is the equivalent for *SectionList: https://reactnative.dev/docs/sectionlist#sections [General][Changed] - Change export type of VirtualizedSectionList and wrap component updates in act in test. [Internal] - Remove FlowFixMe from VirtualizedSectionList Reviewed By: kacieb Differential Revision: D27271493 fbshipit-source-id: f83bdcce9c71c140c8a8fb6f0ecd9bb520fcb85b
1 parent 07d527f commit 5475f29

File tree

8 files changed

+264
-109
lines changed

8 files changed

+264
-109
lines changed

Libraries/Lists/FlatList.js

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
ViewabilityConfigCallbackPair,
2626
} from './ViewabilityHelper';
2727
import type {RenderItemType, RenderItemProps} from './VirtualizedList';
28+
import {keyExtractor as defaultKeyExtractor} from './VirtualizeUtils';
2829

2930
type RequiredProps<ItemT> = {|
3031
/**
@@ -120,7 +121,7 @@ type OptionalProps<ItemT> = {|
120121
* and as the react key to track item re-ordering. The default extractor checks `item.key`, then
121122
* falls back to using the index, like React does.
122123
*/
123-
keyExtractor: (item: ItemT, index: number) => string,
124+
keyExtractor?: ?(item: ItemT, index: number) => string,
124125
/**
125126
* Multiple columns can only be rendered with `horizontal={false}` and will zig-zag like a
126127
* `flexWrap` layout. Items should all be the same height - masonry layouts are not supported.
@@ -156,7 +157,6 @@ export type Props<ItemT> = {
156157
};
157158

158159
const defaultProps = {
159-
...VirtualizedList.defaultProps,
160160
numColumns: 1,
161161
/**
162162
* Enabling this prop on Android greatly improves scrolling performance with no known issues.
@@ -503,28 +503,33 @@ class FlatList<ItemT> extends React.PureComponent<Props<ItemT>, void> {
503503
};
504504

505505
_keyExtractor = (items: ItemT | Array<ItemT>, index: number) => {
506-
const {keyExtractor, numColumns} = this.props;
506+
const {numColumns} = this.props;
507+
const keyExtractor = this.props.keyExtractor ?? defaultKeyExtractor;
508+
507509
if (numColumns > 1) {
508-
invariant(
509-
Array.isArray(items),
510-
'FlatList: Encountered internal consistency error, expected each item to consist of an ' +
511-
'array with 1-%s columns; instead, received a single item.',
512-
numColumns,
513-
);
514-
return (
515-
items
516-
// $FlowFixMe[incompatible-call]
517-
.map((it, kk) => keyExtractor(it, index * numColumns + kk))
518-
.join(':')
519-
);
510+
if (Array.isArray(items)) {
511+
return items
512+
.map((item, kk) =>
513+
keyExtractor(((item: $FlowFixMe): ItemT), index * numColumns + kk),
514+
)
515+
.join(':');
516+
} else {
517+
invariant(
518+
Array.isArray(items),
519+
'FlatList: Encountered internal consistency error, expected each item to consist of an ' +
520+
'array with 1-%s columns; instead, received a single item.',
521+
numColumns,
522+
);
523+
}
520524
} else {
521525
// $FlowFixMe Can't call keyExtractor with an array
522526
return keyExtractor(items, index);
523527
}
524528
};
525529

526530
_pushMultiColumnViewable(arr: Array<ViewToken>, v: ViewToken): void {
527-
const {numColumns, keyExtractor} = this.props;
531+
const {numColumns} = this.props;
532+
const keyExtractor = this.props.keyExtractor ?? defaultKeyExtractor;
528533
v.item.forEach((item, ii) => {
529534
invariant(v.index != null, 'Missing index!');
530535
const index = v.index * numColumns + ii;

Libraries/Lists/SectionList.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ type OptionalProps<SectionT: SectionBase<any>> = {|
7878
* falls back to using the index, like react does. Note that this sets keys for each item, but
7979
* each overall section still needs its own key.
8080
*/
81-
keyExtractor: (item: Item, index: number) => string,
81+
keyExtractor?: ?(item: Item, index: number) => string,
8282
/**
8383
* Called once when the scroll position gets within `onEndReachedThreshold` of the rendered
8484
* content.
@@ -105,6 +105,10 @@ export type Props<SectionT> = {|
105105
VirtualizedSectionListProps<SectionT>,
106106
'renderItem',
107107
>,
108+
keyExtractor: $PropertyType<
109+
VirtualizedSectionListProps<SectionT>,
110+
'keyExtractor',
111+
>,
108112
...
109113
},
110114
>,
@@ -113,7 +117,6 @@ export type Props<SectionT> = {|
113117
|};
114118

115119
const defaultProps = {
116-
...VirtualizedSectionList.defaultProps,
117120
stickySectionHeadersEnabled: Platform.OS === 'ios',
118121
};
119122

Libraries/Lists/VirtualizeUtils.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,13 @@ export function computeWindowedRenderLimits(
233233
}
234234
return {first, last};
235235
}
236+
237+
export function keyExtractor(item: any, index: number): string {
238+
if (typeof item === 'object' && item?.key != null) {
239+
return item.key;
240+
}
241+
if (typeof item === 'object' && item?.id != null) {
242+
return item.id;
243+
}
244+
return String(index);
245+
}

Libraries/Lists/VirtualizedList.js

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ const infoLog = require('../Utilities/infoLog');
2222
const invariant = require('invariant');
2323
import VirtualizedListInjection from './VirtualizedListInjection';
2424

25-
import {computeWindowedRenderLimits} from './VirtualizeUtils';
25+
import {
26+
keyExtractor as defaultKeyExtractor,
27+
computeWindowedRenderLimits,
28+
} from './VirtualizeUtils';
2629

2730
import * as React from 'react';
2831
import type {ScrollResponderType} from '../Components/ScrollView/ScrollView';
@@ -132,7 +135,7 @@ type OptionalProps = {|
132135
* Reverses the direction of scroll. Uses scale transforms of -1.
133136
*/
134137
inverted?: ?boolean,
135-
keyExtractor: (item: Item, index: number) => string,
138+
keyExtractor?: ?(item: Item, index: number) => string,
136139
/**
137140
* Each cell is rendered using this element. Can be a React Component Class,
138141
* or a render function. Defaults to using View.
@@ -303,10 +306,6 @@ type Props = {|
303306
...OptionalProps,
304307
|};
305308

306-
type DefaultProps = {|
307-
keyExtractor: (item: Item, index: number) => string,
308-
|};
309-
310309
let _usedIndexForKey = false;
311310
let _keylessItemComponentName: string = '';
312311

@@ -315,26 +314,37 @@ type State = {
315314
last: number,
316315
};
317316

317+
/**
318+
* Default Props Helper Functions
319+
* Use the following helper functions for default values
320+
*/
321+
322+
// horizontalOrDefault(this.props.horizontal)
318323
function horizontalOrDefault(horizontal: ?boolean) {
319324
return horizontal ?? false;
320325
}
321326

327+
// initialNumToRenderOrDefault(this.props.initialNumToRenderOrDefault)
322328
function initialNumToRenderOrDefault(initialNumToRender: ?number) {
323329
return initialNumToRender ?? 10;
324330
}
325331

332+
// maxToRenderPerBatchOrDefault(this.props.maxToRenderPerBatch)
326333
function maxToRenderPerBatchOrDefault(maxToRenderPerBatch: ?number) {
327334
return maxToRenderPerBatch ?? 10;
328335
}
329336

337+
// onEndReachedThresholdOrDefault(this.props.onEndReachedThreshold)
330338
function onEndReachedThresholdOrDefault(onEndReachedThreshold: ?number) {
331339
return onEndReachedThreshold ?? 2;
332340
}
333341

342+
// scrollEventThrottleOrDefault(this.props.scrollEventThrottle)
334343
function scrollEventThrottleOrDefault(scrollEventThrottle: ?number) {
335344
return scrollEventThrottle ?? 50;
336345
}
337346

347+
// windowSizeOrDefault(this.props.windowSize)
338348
function windowSizeOrDefault(windowSize: ?number) {
339349
return windowSize ?? 21;
340350
}
@@ -365,6 +375,7 @@ function windowSizeOrDefault(windowSize: ?number) {
365375
* and we are working on improving it behind the scenes.
366376
* - By default, the list looks for a `key` or `id` prop on each item and uses that for the React key.
367377
* Alternatively, you can provide a custom `keyExtractor` prop.
378+
* - As an effort to remove defaultProps, use helper functions when referencing certain props
368379
*
369380
*/
370381
class VirtualizedList extends React.PureComponent<Props, State> {
@@ -580,22 +591,6 @@ class VirtualizedList extends React.PureComponent<Props, State> {
580591
}
581592
}
582593

583-
static defaultProps: DefaultProps = {
584-
keyExtractor: (item: Item, index: number) => {
585-
if (item.key != null) {
586-
return item.key;
587-
}
588-
if (item.id != null) {
589-
return item.id;
590-
}
591-
_usedIndexForKey = true;
592-
if (item.type && item.type.displayName) {
593-
_keylessItemComponentName = item.type.displayName;
594-
}
595-
return String(index);
596-
},
597-
};
598-
599594
_getCellKey(): string {
600595
return this.context?.cellKey || 'rootList';
601596
}
@@ -804,15 +799,14 @@ class VirtualizedList extends React.PureComponent<Props, State> {
804799
getItem,
805800
getItemCount,
806801
horizontal,
807-
keyExtractor,
808802
} = this.props;
809803
const stickyOffset = this.props.ListHeaderComponent ? 1 : 0;
810804
const end = getItemCount(data) - 1;
811805
let prevCellKey;
812806
last = Math.min(end, last);
813807
for (let ii = first; ii <= last; ii++) {
814808
const item = getItem(data, ii);
815-
const key = keyExtractor(item, ii);
809+
const key = this._keyExtractor(item, ii);
816810
this._indicesToKeys.set(ii, key);
817811
if (stickyIndicesFromProps.has(ii + stickyOffset)) {
818812
stickyHeaderIndices.push(cells.length);
@@ -864,6 +858,21 @@ class VirtualizedList extends React.PureComponent<Props, State> {
864858
_getSpacerKey = (isVertical: boolean): string =>
865859
isVertical ? 'height' : 'width';
866860

861+
_keyExtractor(item: Item, index: number) {
862+
if (this.props.keyExtractor != null) {
863+
return this.props.keyExtractor(item, index);
864+
}
865+
866+
const key = defaultKeyExtractor(item, index);
867+
if (key === String(index)) {
868+
_usedIndexForKey = true;
869+
if (item.type && item.type.displayName) {
870+
_keylessItemComponentName = item.type.displayName;
871+
}
872+
}
873+
return key;
874+
}
875+
867876
render(): React.Node {
868877
if (__DEV__) {
869878
const flatStyles = flattenStyle(this.props.contentContainerStyle);
@@ -1816,9 +1825,9 @@ class VirtualizedList extends React.PureComponent<Props, State> {
18161825
};
18171826

18181827
_createViewToken = (index: number, isViewable: boolean) => {
1819-
const {data, getItem, keyExtractor} = this.props;
1828+
const {data, getItem} = this.props;
18201829
const item = getItem(data, index);
1821-
return {index, item, key: keyExtractor(item, index), isViewable};
1830+
return {index, item, key: this._keyExtractor(item, index), isViewable};
18221831
};
18231832

18241833
_getFrameMetricsApprox = (
@@ -1854,19 +1863,13 @@ class VirtualizedList extends React.PureComponent<Props, State> {
18541863
inLayout?: boolean,
18551864
...
18561865
} => {
1857-
const {
1858-
data,
1859-
getItem,
1860-
getItemCount,
1861-
getItemLayout,
1862-
keyExtractor,
1863-
} = this.props;
1866+
const {data, getItem, getItemCount, getItemLayout} = this.props;
18641867
invariant(
18651868
getItemCount(data) > index,
18661869
'Tried to get frame for out of range index ' + index,
18671870
);
18681871
const item = getItem(data, index);
1869-
let frame = item && this._frames[keyExtractor(item, index)];
1872+
let frame = item && this._frames[this._keyExtractor(item, index)];
18701873
if (!frame || frame.index !== index) {
18711874
if (getItemLayout) {
18721875
frame = getItemLayout(data, index);

0 commit comments

Comments
 (0)