Gracefully handle out-of-bounds initialScrollIndex

Summary:
Changelog:
[General][Fixed] - Gracefully handle out-of-bounds initialScrollIndex

Reviewed By: rshest

Differential Revision: D43672964

fbshipit-source-id: dbd9007c538015fc586e573d268135b7557dc908
This commit is contained in:
Nick Gerleman 2023-03-02 16:13:04 -08:00 коммит произвёл Facebook GitHub Bot
Родитель 31b4550a17
Коммит aab9df3710
4 изменённых файлов: 129 добавлений и 46 удалений

Просмотреть файл

@ -50,6 +50,7 @@ import {
keyExtractor as defaultKeyExtractor,
} from './VirtualizeUtils';
import invariant from 'invariant';
import nullthrows from 'nullthrows';
import * as React from 'react';
export type {RenderItemProps, RenderItemType, Separators};
@ -420,21 +421,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
constructor(props: Props) {
super(props);
invariant(
// $FlowFixMe[prop-missing]
!props.onScroll || !props.onScroll.__isNative,
'Components based on VirtualizedList must be wrapped with Animated.createAnimatedComponent ' +
'to support native onScroll events with useNativeDriver',
);
invariant(
windowSizeOrDefault(props.windowSize) > 0,
'VirtualizedList: The windowSize prop must be present and set to a value greater than 0.',
);
invariant(
props.getItemCount,
'VirtualizedList: The "getItemCount" prop must be provided',
);
this.checkProps(props);
this._fillRateHelper = new FillRateHelper(this._getFrameMetrics);
this._updateCellsToRenderBatcher = new Batchinator(
@ -459,11 +446,6 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
}
}
invariant(
!this.context,
'Unexpectedly saw VirtualizedListContext available in ctor',
);
const initialRenderRegion = VirtualizedList._initialRenderRegion(props);
this.state = {
@ -472,6 +454,53 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
};
}
checkProps(props: Props) {
const {onScroll, windowSize, getItemCount, data, initialScrollIndex} =
props;
invariant(
// $FlowFixMe[prop-missing]
!onScroll || !onScroll.__isNative,
'Components based on VirtualizedList must be wrapped with Animated.createAnimatedComponent ' +
'to support native onScroll events with useNativeDriver',
);
invariant(
windowSizeOrDefault(windowSize) > 0,
'VirtualizedList: The windowSize prop must be present and set to a value greater than 0.',
);
invariant(
getItemCount,
'VirtualizedList: The "getItemCount" prop must be provided',
);
const itemCount = getItemCount(data);
if (
initialScrollIndex != null &&
(initialScrollIndex < 0 ||
(itemCount > 0 && initialScrollIndex >= itemCount)) &&
!this._hasWarned.initialScrollIndex
) {
console.warn(
`initialScrollIndex "${initialScrollIndex}" is not valid (list has ${itemCount} items)`,
);
this._hasWarned.initialScrollIndex = true;
}
if (__DEV__ && !this._hasWarned.flexWrap) {
// $FlowFixMe[underconstrained-implicit-instantiation]
const flatStyles = StyleSheet.flatten(this.props.contentContainerStyle);
if (flatStyles != null && flatStyles.flexWrap === 'wrap') {
console.warn(
'`flexWrap: `wrap`` is not supported with the `VirtualizedList` components.' +
'Consider using `numColumns` with `FlatList` instead.',
);
this._hasWarned.flexWrap = true;
}
}
}
static _createRenderMask(
props: Props,
cellsAroundViewport: {first: number, last: number},
@ -518,15 +547,21 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
static _initialRenderRegion(props: Props): {first: number, last: number} {
const itemCount = props.getItemCount(props.data);
const scrollIndex = Math.floor(Math.max(0, props.initialScrollIndex ?? 0));
return {
first: scrollIndex,
last:
const firstCellIndex = Math.max(
0,
Math.min(itemCount - 1, Math.floor(props.initialScrollIndex ?? 0)),
);
const lastCellIndex =
Math.min(
itemCount,
scrollIndex + initialNumToRenderOrDefault(props.initialNumToRender),
) - 1,
firstCellIndex + initialNumToRenderOrDefault(props.initialNumToRender),
) - 1;
return {
first: firstCellIndex,
last: lastCellIndex,
};
}
@ -807,16 +842,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
}
render(): React.Node {
if (__DEV__) {
// $FlowFixMe[underconstrained-implicit-instantiation]
const flatStyles = StyleSheet.flatten(this.props.contentContainerStyle);
if (flatStyles != null && flatStyles.flexWrap === 'wrap') {
console.warn(
'`flexWrap: `wrap`` is not supported with the `VirtualizedList` components.' +
'Consider using `numColumns` with `FlatList` instead.',
);
}
}
this.checkProps(this.props);
const {ListEmptyComponent, ListFooterComponent, ListHeaderComponent} =
this.props;
const {data, horizontal} = this.props;
@ -1507,10 +1533,17 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
!this._hasTriggeredInitialScrollToIndex
) {
if (this.props.contentOffset == null) {
if (
this.props.initialScrollIndex <
this.props.getItemCount(this.props.data)
) {
this.scrollToIndex({
animated: false,
index: this.props.initialScrollIndex,
index: nullthrows(this.props.initialScrollIndex),
});
} else {
this.scrollToEnd({animated: false});
}
}
this._hasTriggeredInitialScrollToIndex = true;
}

Просмотреть файл

@ -825,10 +825,12 @@ it('unmounts sticky headers moved below viewport', () => {
expect(component).toMatchSnapshot();
});
it('gracefully handles negaitve initialScrollIndex', () => {
it('gracefully handles negative initialScrollIndex', () => {
const items = generateItems(10);
const ITEM_HEIGHT = 10;
const mockWarn = jest.spyOn(console, 'warn').mockImplementation(() => {});
const component = ReactTestRenderer.create(
<VirtualizedList
initialScrollIndex={-1}
@ -838,9 +840,56 @@ it('gracefully handles negaitve initialScrollIndex', () => {
/>,
);
// Existing code assumes we handle this in some way. Do something reasonable
// here.
expect(mockWarn).toHaveBeenCalledTimes(1);
ReactTestRenderer.act(() => {
simulateLayout(component, {
viewport: {width: 10, height: 50},
content: {width: 10, height: 100},
});
performAllBatches();
});
expect(component).toMatchSnapshot();
expect(mockWarn).toHaveBeenCalledTimes(1);
mockWarn.mockRestore();
});
it('gracefully handles too large initialScrollIndex', () => {
const items = generateItems(10);
const ITEM_HEIGHT = 10;
const listRef = React.createRef();
const mockWarn = jest.spyOn(console, 'warn').mockImplementation(() => {});
const component = ReactTestRenderer.create(
<VirtualizedList
ref={listRef}
initialScrollIndex={15}
initialNumToRender={4}
{...baseItemProps(items)}
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
/>,
);
expect(mockWarn).toHaveBeenCalledTimes(1);
listRef.current.scrollToEnd = jest.fn();
ReactTestRenderer.act(() => {
simulateLayout(component, {
viewport: {width: 10, height: 50},
content: {width: 10, height: 100},
});
performAllBatches();
});
expect(mockWarn).toHaveBeenCalledTimes(1);
mockWarn.mockRestore();
expect(listRef.current.scrollToEnd).toHaveBeenLastCalledWith({
animated: false,
});
});
it('renders offset cells in initial render when initialScrollIndex set', () => {

Просмотреть файл

@ -3036,7 +3036,7 @@ exports[`expands render area by maxToRenderPerBatch on tick 1`] = `
</RCTScrollView>
`;
exports[`gracefully handles negaitve initialScrollIndex 1`] = `
exports[`gracefully handles negative initialScrollIndex 1`] = `
<RCTScrollView
data={
Array [

Просмотреть файл

@ -9,7 +9,8 @@
},
"license": "MIT",
"dependencies": {
"invariant": "^2.2.4"
"invariant": "^2.2.4",
"nullthrows": "^1.1.1"
},
"devDependencies": {
"react-test-renderer": "18.2.0"