From 2265f04334c1fa395674b58184e0ad27d6ba0047 Mon Sep 17 00:00:00 2001
From: Antoine Doubovetzky <antoined@bam.tech>
Date: Sun, 3 Apr 2022 11:14:30 +0200
Subject: [PATCH 1/3] :white_check_mark: (VirtualizedList) add broken test case
 based on issue example

This test case is based on this issue: https://github.com/facebook/react-native/issues/33529
---
 .../Lists/__tests__/VirtualizedList-test.js   | 49 +++++++++++++++
 .../VirtualizedList-test.js.snap              | 62 +++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js
index a7589992564214..ad3a52e37b62e8 100644
--- a/Libraries/Lists/__tests__/VirtualizedList-test.js
+++ b/Libraries/Lists/__tests__/VirtualizedList-test.js
@@ -928,6 +928,55 @@ it('adjusts render area with non-zero initialScrollIndex after scrolled', () =>
   expect(component).toMatchSnapshot();
 });
 
+it('renders new items when data is updated with non-zero initialScrollIndex', () => {
+  const items = generateItems(2);
+  const ITEM_HEIGHT = 10;
+
+  let component;
+  ReactTestRenderer.act(() => {
+    component = ReactTestRenderer.create(
+      <VirtualizedList
+        initialNumToRender={5}
+        initialScrollIndex={1}
+        windowSize={10}
+        maxToRenderPerBatch={10}
+        {...baseItemProps(items)}
+        {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
+      />,
+    );
+  });
+
+  ReactTestRenderer.act(() => {
+    simulateLayout(component, {
+      viewport: {width: 10, height: 50},
+      content: {width: 10, height: 200},
+    });
+    performAllBatches();
+  });
+
+  const newItems = generateItems(4);
+
+  ReactTestRenderer.act(() => {
+    component.update(
+      <VirtualizedList
+        initialNumToRender={5}
+        initialScrollIndex={1}
+        windowSize={10}
+        maxToRenderPerBatch={10}
+        {...baseItemProps(newItems)}
+        {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
+      />,
+    );
+  });
+
+  ReactTestRenderer.act(() => {
+    performAllBatches();
+  });
+
+  // We expect all the items to be rendered
+  expect(component).toMatchSnapshot();
+});
+
 it('renders initialNumToRender cells when virtualization disabled', () => {
   const items = generateItems(10);
   const ITEM_HEIGHT = 10;
diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
index ad93703101b2ad..3a467ba9de538c 100644
--- a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
+++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
@@ -3250,6 +3250,68 @@ exports[`renders items before initialScrollIndex on first batch tick when virtua
 </RCTScrollView>
 `;
 
+exports[`renders new items when data is updated with non-zero initialScrollIndex 1`] = `
+<RCTScrollView
+  data={
+    Array [
+      Object {
+        "key": 0,
+      },
+      Object {
+        "key": 1,
+      },
+      Object {
+        "key": 2,
+      },
+      Object {
+        "key": 3,
+      },
+    ]
+  }
+  getItem={[Function]}
+  getItemCount={[Function]}
+  getItemLayout={[Function]}
+  initialNumToRender={5}
+  initialScrollIndex={1}
+  maxToRenderPerBatch={10}
+  onContentSizeChange={[Function]}
+  onLayout={[Function]}
+  onMomentumScrollBegin={[Function]}
+  onMomentumScrollEnd={[Function]}
+  onScroll={[Function]}
+  onScrollBeginDrag={[Function]}
+  onScrollEndDrag={[Function]}
+  renderItem={[Function]}
+  scrollEventThrottle={50}
+  stickyHeaderIndices={Array []}
+  windowSize={10}
+>
+  <View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={0}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={1}
+      />
+    </View>
+    <View
+      style={
+        Object {
+          "height": 20,
+        }
+      }
+    />
+  </View>
+</RCTScrollView>
+`;
+
 exports[`renders no spacers up to initialScrollIndex on first render when virtualization disabled 1`] = `
 <RCTScrollView
   data={

From 2335afbeea0fa9334db0b339e9c043f8578ad7f7 Mon Sep 17 00:00:00 2001
From: Antoine Doubovetzky <antoined@bam.tech>
Date: Sun, 3 Apr 2022 11:28:02 +0200
Subject: [PATCH 2/3] =?UTF-8?q?:bug:=20(VirtualizedList)=20fix=20Virtualiz?=
 =?UTF-8?q?edList=20with=20initialScrollIndex=20not=20rendering=20all=20it?=
 =?UTF-8?q?ems=20when=20da=E2=80=A6?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Use this._hasDoneInitialScroll instead of this._scrollMetrics.offset to determine if the initial
scroll is done. The offset can sometimes be 0 even after the scrollToIndex method is called to
scroll to the initialScrollIndex: for example when the content does not fill the list.
---
 Libraries/Lists/VirtualizedList.js            |   2 +-
 .../VirtualizedList-test.js.snap              | 134 +++++++++++++++---
 2 files changed, 117 insertions(+), 19 deletions(-)

diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js
index 0a02fb1ef1567f..77b69cd72ac976 100644
--- a/Libraries/Lists/VirtualizedList.js
+++ b/Libraries/Lists/VirtualizedList.js
@@ -1754,7 +1754,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
           // we'll wipe out the initialNumToRender rendered elements starting at initialScrollIndex.
           // So let's wait until we've scrolled the view to the right place. And until then,
           // we will trust the initialScrollIndex suggestion.
-          if (!this.props.initialScrollIndex || this._scrollMetrics.offset) {
+          if (!this.props.initialScrollIndex || this._hasDoneInitialScroll) {
             newState = computeWindowedRenderLimits(
               this.props.data,
               this.props.getItemCount,
diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
index 3a467ba9de538c..037d1d769fe3f5 100644
--- a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
+++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
@@ -2293,12 +2293,12 @@ exports[`does not adjust render area with non-zero initialScrollIndex until scro
 >
   <View>
     <View
-      style={
-        Object {
-          "height": 10,
-        }
-      }
-    />
+      style={null}
+    >
+      <MockCellItem
+        value={0}
+      />
+    </View>
     <View
       style={null}
     >
@@ -2335,12 +2335,103 @@ exports[`does not adjust render area with non-zero initialScrollIndex until scro
       />
     </View>
     <View
-      style={
-        Object {
-          "height": 140,
-        }
-      }
-    />
+      style={null}
+    >
+      <MockCellItem
+        value={6}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={7}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={8}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={9}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={10}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={11}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={12}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={13}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={14}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={15}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={16}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={17}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={18}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={19}
+      />
+    </View>
   </View>
 </RCTScrollView>
 `;
@@ -3302,12 +3393,19 @@ exports[`renders new items when data is updated with non-zero initialScrollIndex
       />
     </View>
     <View
-      style={
-        Object {
-          "height": 20,
-        }
-      }
-    />
+      style={null}
+    >
+      <MockCellItem
+        value={2}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={3}
+      />
+    </View>
   </View>
 </RCTScrollView>
 `;

From 5eb2a0a02fad519199c57f688dae0b207498b549 Mon Sep 17 00:00:00 2001
From: Antoine Doubovetzky <antoined@bam.tech>
Date: Sun, 3 Apr 2022 11:44:30 +0200
Subject: [PATCH 3/3] :fire: (VirtualizedList-test) remove unrelevant test case

This test case is unrelevant because we do not need to simulate a scroll to check that the render
area has been adjusted, and this behavior is already tested by "adjusts render area with non-zero
initialScrollIndex".
---
 .../Lists/__tests__/VirtualizedList-test.js   |  36 +--
 .../VirtualizedList-test.js.snap              | 231 +-----------------
 2 files changed, 2 insertions(+), 265 deletions(-)

diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js
index ad3a52e37b62e8..9f902f180c7fbc 100644
--- a/Libraries/Lists/__tests__/VirtualizedList-test.js
+++ b/Libraries/Lists/__tests__/VirtualizedList-test.js
@@ -862,40 +862,7 @@ it('does not adjust render area until content area layed out', () => {
   expect(component).toMatchSnapshot();
 });
 
-it('does not adjust render area with non-zero initialScrollIndex until scrolled', () => {
-  const items = generateItems(20);
-  const ITEM_HEIGHT = 10;
-
-  let component;
-  ReactTestRenderer.act(() => {
-    component = ReactTestRenderer.create(
-      <VirtualizedList
-        initialNumToRender={5}
-        initialScrollIndex={1}
-        windowSize={10}
-        maxToRenderPerBatch={10}
-        {...baseItemProps(items)}
-        {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
-      />,
-    );
-  });
-
-  ReactTestRenderer.act(() => {
-    simulateLayout(component, {
-      viewport: {width: 10, height: 50},
-      content: {width: 10, height: 200},
-    });
-    performAllBatches();
-  });
-
-  // Layout information from before the time we scroll to initial index may not
-  // correspond to the area "initialScrollIndex" points to. Expect only the 5
-  // initial items (starting at initialScrollIndex) to be rendered after
-  // processing all batch work, even though the windowSize allows for more.
-  expect(component).toMatchSnapshot();
-});
-
-it('adjusts render area with non-zero initialScrollIndex after scrolled', () => {
+it('adjusts render area with non-zero initialScrollIndex', () => {
   const items = generateItems(20);
   const ITEM_HEIGHT = 10;
 
@@ -919,7 +886,6 @@ it('adjusts render area with non-zero initialScrollIndex after scrolled', () =>
       content: {width: 10, height: 200},
     });
 
-    simulateScroll(component, {x: 0, y: 10});
     performAllBatches();
   });
 
diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
index 037d1d769fe3f5..7c8cbb8edd6f5f 100644
--- a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
+++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
@@ -1600,7 +1600,7 @@ exports[`VirtualizedList warns if both renderItem or ListItemComponent are speci
 </RCTScrollView>
 `;
 
-exports[`adjusts render area with non-zero initialScrollIndex after scrolled 1`] = `
+exports[`adjusts render area with non-zero initialScrollIndex 1`] = `
 <RCTScrollView
   data={
     Array [
@@ -2207,235 +2207,6 @@ exports[`does not adjust render area until content area layed out 1`] = `
 </RCTScrollView>
 `;
 
-exports[`does not adjust render area with non-zero initialScrollIndex until scrolled 1`] = `
-<RCTScrollView
-  data={
-    Array [
-      Object {
-        "key": 0,
-      },
-      Object {
-        "key": 1,
-      },
-      Object {
-        "key": 2,
-      },
-      Object {
-        "key": 3,
-      },
-      Object {
-        "key": 4,
-      },
-      Object {
-        "key": 5,
-      },
-      Object {
-        "key": 6,
-      },
-      Object {
-        "key": 7,
-      },
-      Object {
-        "key": 8,
-      },
-      Object {
-        "key": 9,
-      },
-      Object {
-        "key": 10,
-      },
-      Object {
-        "key": 11,
-      },
-      Object {
-        "key": 12,
-      },
-      Object {
-        "key": 13,
-      },
-      Object {
-        "key": 14,
-      },
-      Object {
-        "key": 15,
-      },
-      Object {
-        "key": 16,
-      },
-      Object {
-        "key": 17,
-      },
-      Object {
-        "key": 18,
-      },
-      Object {
-        "key": 19,
-      },
-    ]
-  }
-  getItem={[Function]}
-  getItemCount={[Function]}
-  getItemLayout={[Function]}
-  initialNumToRender={5}
-  initialScrollIndex={1}
-  maxToRenderPerBatch={10}
-  onContentSizeChange={[Function]}
-  onLayout={[Function]}
-  onMomentumScrollBegin={[Function]}
-  onMomentumScrollEnd={[Function]}
-  onScroll={[Function]}
-  onScrollBeginDrag={[Function]}
-  onScrollEndDrag={[Function]}
-  renderItem={[Function]}
-  scrollEventThrottle={50}
-  stickyHeaderIndices={Array []}
-  windowSize={10}
->
-  <View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={0}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={1}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={2}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={3}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={4}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={5}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={6}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={7}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={8}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={9}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={10}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={11}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={12}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={13}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={14}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={15}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={16}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={17}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={18}
-      />
-    </View>
-    <View
-      style={null}
-    >
-      <MockCellItem
-        value={19}
-      />
-    </View>
-  </View>
-</RCTScrollView>
-`;
-
 exports[`does not over-render when there is less than initialNumToRender cells 1`] = `
 <RCTScrollView
   data={