Allow empty cell ranges in CellRenderMask.addCells()

Summary:
VirtualizedList state is represented in terms of [first, last] ranges, where it is possible to express a zero-cell range by having [n, n-1]. This includes some awkward examples, like [0, -1] being valid.

CellRenderMask assumes `addCells` is called with at least one cell, with VirtualizedList previously guarding against adding the no cell case. This guard is present for adding main cell regions, but not for `_initialRenderRegion()`, which can be overridden to have a zero length as well.

This moves the `CellRenderMask` to be permissive of zero-length cell regions, and removes the caller guard.

Changelog:
[Internal][Fixed] - Allow empty cell ranges in CellRenderMask

Reviewed By: rshest

Differential Revision: D38184444

fbshipit-source-id: d2dadfdd9628b24f894126d63b1eb93f6387b877

Libraries/Lists/CellRenderMask.js

@@ -49,12 +49,18 @@ export class CellRenderMask {
     invariant(
       cells.first >= 0 &&
         cells.first < this._numCells &&
-        cells.last >= 0 &&
+        cells.last >= -1 &&
         cells.last < this._numCells &&
-        cells.last >= cells.first,
+        cells.last >= cells.first - 1,
       'CellRenderMask.addCells called with invalid cell range',
     );
 
+    // VirtualizedList uses inclusive ranges, where zero-count states are
+    // possible. E.g. [0, -1] for no cells, starting at 0.
+    if (cells.last < cells.first) {
+      return;
+    }
+
     const [firstIntersect, firstIntersectIdx] = this._findRegion(cells.first);
     const [lastIntersect, lastIntersectIdx] = this._findRegion(cells.last);
 

Libraries/Lists/VirtualizedList_EXPERIMENTAL.js

@@ -779,9 +779,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
     if (itemCount > 0) {
       const allRegions = [cellsAroundViewport, ...(additionalRegions ?? [])];
       for (const region of allRegions) {
-        if (region.last >= region.first) {
-          renderMask.addCells(region);
-        }
+        renderMask.addCells(region);
       }
 
       // The initially rendered cells are retained as part of the

Libraries/Lists/__tests__/CellRenderMask-test.js

@@ -30,8 +30,8 @@ describe('CellRenderMask', () => {
   it('throws when adding invalid cell ranges', () => {
     const renderMask = new CellRenderMask(5);
 
-    expect(() => renderMask.addCells({first: -1, last: 0})).toThrow();
-    expect(() => renderMask.addCells({first: 1, last: 0})).toThrow();
+    expect(() => renderMask.addCells({first: -2, last: -1})).toThrow();
+    expect(() => renderMask.addCells({first: -2, last: 0})).toThrow();
     expect(() => renderMask.addCells({first: 0, last: 5})).toThrow();
     expect(() => renderMask.addCells({first: 6, last: 7})).toThrow();
   });
@@ -76,6 +76,15 @@ describe('CellRenderMask', () => {
     ]);
   });
 
+  it('allows adding empty cell range', () => {
+    const renderMask = new CellRenderMask(5);
+    renderMask.addCells({first: 0, last: -1});
+
+    expect(renderMask.enumerateRegions()).toEqual([
+      {first: 0, last: 4, isSpacer: true},
+    ]);
+  });
+
   it('correctly replaces fragmented cell ranges', () => {
     const renderMask = new CellRenderMask(10);
 

Libraries/Lists/__tests__/VirtualizedList-test.js

@@ -736,6 +736,23 @@ it('renders offset cells in initial render when initialScrollIndex set', () => {
   expect(component).toMatchSnapshot();
 });
 
+it('initially renders nothing when initialNumToRender is 0', () => {
+  const items = generateItems(10);
+  const ITEM_HEIGHT = 10;
+
+  const component = ReactTestRenderer.create(
+    <VirtualizedList
+      initialNumToRender={0}
+      {...baseItemProps(items)}
+      {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
+    />,
+  );
+
+  // Only a spacer should be present (a single item is present in the legacy
+  // implementation)
+  expect(component).toMatchSnapshot();
+});
+
 it('does not over-render when there is less than initialNumToRender cells', () => {
   const items = generateItems(10);
   const ITEM_HEIGHT = 10;

Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap

@@ -2799,6 +2799,76 @@ exports[`gracefully handles negaitve initialScrollIndex 1`] = `
 </RCTScrollView>
 `;
 
+exports[`initially renders nothing when initialNumToRender is 0 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,
+      },
+    ]
+  }
+  getItem={[Function]}
+  getItemCount={[Function]}
+  getItemLayout={[Function]}
+  initialNumToRender={0}
+  onContentSizeChange={[Function]}
+  onLayout={[Function]}
+  onMomentumScrollBegin={[Function]}
+  onMomentumScrollEnd={[Function]}
+  onScroll={[Function]}
+  onScrollBeginDrag={[Function]}
+  onScrollEndDrag={[Function]}
+  renderItem={[Function]}
+  scrollEventThrottle={50}
+  stickyHeaderIndices={Array []}
+>
+  <View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={0}
+      />
+    </View>
+    <View
+      style={
+        Object {
+          "height": 90,
+        }
+      }
+    />
+  </View>
+</RCTScrollView>
+`;
+
 exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = `
 <RCTScrollView
   data={

Libraries/Lists/__tests__/__snapshots__/VirtualizedList_EXPERIMENTAL-test.js.snap

@@ -3587,6 +3587,69 @@ exports[`gracefully handles negaitve initialScrollIndex 1`] = `
 </RCTScrollView>
 `;
 
+exports[`initially renders nothing when initialNumToRender is 0 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,
+      },
+    ]
+  }
+  getItem={[Function]}
+  getItemCount={[Function]}
+  getItemLayout={[Function]}
+  initialNumToRender={0}
+  onContentSizeChange={[Function]}
+  onLayout={[Function]}
+  onMomentumScrollBegin={[Function]}
+  onMomentumScrollEnd={[Function]}
+  onScroll={[Function]}
+  onScrollBeginDrag={[Function]}
+  onScrollEndDrag={[Function]}
+  renderItem={[Function]}
+  scrollEventThrottle={50}
+  stickyHeaderIndices={Array []}
+>
+  <View>
+    <View
+      style={
+        Object {
+          "height": 100,
+        }
+      }
+    />
+  </View>
+</RCTScrollView>
+`;
+
 exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = `
 <RCTScrollView
   data={