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

Author: NickGerleman

packages/virtualized-lists/Lists/VirtualizedList.js

@@ -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));
+
+    const firstCellIndex = Math.max(
+      0,
+      Math.min(itemCount - 1, Math.floor(props.initialScrollIndex ?? 0)),
+    );
+
+    const lastCellIndex =
+      Math.min(
+        itemCount,
+        firstCellIndex + initialNumToRenderOrDefault(props.initialNumToRender),
+      ) - 1;
 
     return {
-      first: scrollIndex,
-      last:
-        Math.min(
-          itemCount,
-          scrollIndex + initialNumToRenderOrDefault(props.initialNumToRender),
-        ) - 1,
+      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) {
-        this.scrollToIndex({
-          animated: false,
-          index: this.props.initialScrollIndex,
-        });
+        if (
+          this.props.initialScrollIndex <
+          this.props.getItemCount(this.props.data)
+        ) {
+          this.scrollToIndex({
+            animated: false,
+            index: nullthrows(this.props.initialScrollIndex),
+          });
+        } else {
+          this.scrollToEnd({animated: false});
+        }
       }
       this._hasTriggeredInitialScrollToIndex = true;
     }

packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js

@@ -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', () => {

packages/virtualized-lists/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap

@@ -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 [

packages/virtualized-lists/package.json

@@ -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"