Merge pull request #32 from nihgwu/neo/forceUpdate

add shouldForceUpdate to resetAfterIndex
bvaughn · Aug 4, 2018 · 1aa9f8c · 1aa9f8c
2 parents 4c2f393 + dce5b6d
commit 1aa9f8c
Show file tree

Hide file tree

Showing 6 changed files with 190 additions and 39 deletions.
diff --git a/src/VariableSizeGrid.js b/src/VariableSizeGrid.js
@@ -396,20 +396,28 @@ const VariableSizeGrid = createGridComponent({
       rowMetadataMap: {},
     };
 
-    instance.resetAfterColumnIndex = (columnIndex: number) => {
-      this.resetAfterIndices({ columnIndex });
+    instance.resetAfterColumnIndex = (
+      columnIndex: number,
+      shouldForceUpdate?: boolean = true
+    ) => {
+      this.resetAfterIndices({ columnIndex, shouldForceUpdate });
     };
 
-    instance.resetAfterRowIndex = (rowIndex: number) => {
-      this.resetAfterIndices({ rowIndex });
+    instance.resetAfterRowIndex = (
+      rowIndex: number,
+      shouldForceUpdate?: boolean = true
+    ) => {
+      this.resetAfterIndices({ rowIndex, shouldForceUpdate });
     };
 
     instance.resetAfterIndices = ({
       columnIndex,
       rowIndex,
+      shouldForceUpdate = true,
     }: {
       columnIndex?: number,
       rowIndex?: number,
+      shouldForceUpdate: boolean,
     }) => {
       if (typeof columnIndex === 'number') {
         instanceProps.lastMeasuredColumnIndex = Math.min(
@@ -429,7 +437,10 @@ const VariableSizeGrid = createGridComponent({
       // It seems an unnecessary optimization.
       // It's unlikely that resetAfterIndex() will be called while a user is scrolling.
       instance._itemStyleCache = {};
-      instance.forceUpdate();
+
+      if (shouldForceUpdate) {
+        instance.forceUpdate();
+      }
     };
 
     return instanceProps;

diff --git a/src/VariableSizeList.js b/src/VariableSizeList.js
@@ -251,7 +251,10 @@ const VariableSizeList = createListComponent({
       lastMeasuredIndex: -1,
     };
 
-    instance.resetAfterIndex = (index: number) => {
+    instance.resetAfterIndex = (
+      index: number,
+      shouldForceUpdate?: boolean = true
+    ) => {
       instanceProps.lastMeasuredIndex = Math.min(
         instanceProps.lastMeasuredIndex,
         index - 1
@@ -262,7 +265,10 @@ const VariableSizeList = createListComponent({
       // It seems an unnecessary optimization.
       // It's unlikely that resetAfterIndex() will be called while a user is scrolling.
       instance._itemStyleCache = {};
-      instance.forceUpdate();
+
+      if (shouldForceUpdate) {
+        instance.forceUpdate();
+      }
     };
 
     return instanceProps;

diff --git a/src/__tests__/VariableSizeGrid.js b/src/__tests__/VariableSizeGrid.js
@@ -245,6 +245,63 @@ describe('VariableSizeGrid', () => {
       expect(scrollContainer.props.style.width).toEqual(625);
     });
 
+    it('should delay the recalculation of the estimated total size if shouldForceUpdate is false', () => {
+      const rendered = ReactTestRenderer.create(
+        <VariableSizeGrid
+          {...defaultProps}
+          estimatedColumnWidth={30}
+          estimatedRowHeight={30}
+          overscanCount={1}
+          columnWidth={index => 50}
+          rowHeight={index => 25}
+        />
+      );
+      const scrollContainer = findScrollContainer(rendered);
+      // The estimated total height should be (100 + 25 * 1 + 30 * 15)px = 575px.
+      // The estimated total width should be (200 + 50 * 1 + 30 * 5)px = 400px.
+      expect(scrollContainer.props.style.height).toEqual(575);
+      expect(scrollContainer.props.style.width).toEqual(400);
+      // Supplying new item sizes alone should not impact anything.
+      // Although the grid get re-rendered by passing inline functions,
+      // but it still use the cached metrics to calculate the estimated size.
+      rendered.update(
+        <VariableSizeGrid
+          {...defaultProps}
+          estimatedColumnWidth={30}
+          estimatedRowHeight={30}
+          overscanCount={1}
+          columnWidth={index => 40}
+          rowHeight={index => 20}
+        />
+      );
+      expect(scrollContainer.props.style.height).toEqual(575);
+      expect(scrollContainer.props.style.width).toEqual(400);
+      // Reset calculation cache but don't re-render the grid,
+      // the estimated total size should stay the same.
+      rendered.getInstance().resetAfterIndices({
+        columnIndex: 0,
+        rowIndex: 0,
+        shouldForceUpdate: false,
+      });
+      expect(scrollContainer.props.style.height).toEqual(575);
+      expect(scrollContainer.props.style.width).toEqual(400);
+      // Pass inline function to make the grid re-render.
+      rendered.update(
+        <VariableSizeGrid
+          {...defaultProps}
+          estimatedColumnWidth={30}
+          estimatedRowHeight={30}
+          overscanCount={1}
+          columnWidth={index => 40}
+          rowHeight={index => 20}
+        />
+      );
+      // The estimated total height should be (100 + 20 * 1 + 30 * 14)px = 540px.
+      // The estimated total width should be (200 + 40 * 1 + 30 * 4)px = 360px.
+      expect(scrollContainer.props.style.height).toEqual(540);
+      expect(scrollContainer.props.style.width).toEqual(360);
+    });
+
     it('should re-render items after the specified index with updated styles', () => {
       const columnWidth = jest.fn(() => 75);
       const rowHeight = jest.fn(() => 35);

diff --git a/src/__tests__/VariableSizeList.js b/src/__tests__/VariableSizeList.js
@@ -190,6 +190,47 @@ describe('VariableSizeList', () => {
       expect(scrollContainer.props.style.height).toEqual(750);
     });
 
+    it('should delay the recalculation of the estimated total size if shouldForceUpdate is false', () => {
+      const rendered = ReactTestRenderer.create(
+        <VariableSizeList
+          {...defaultProps}
+          estimatedItemSize={30}
+          overscanCount={1}
+          itemSize={index => 25}
+        />
+      );
+      const scrollContainer = findScrollContainer(rendered);
+      // The estimated total size should be (100 + 25 * 1 + 30 * 15)px = 575px.
+      expect(scrollContainer.props.style.height).toEqual(575);
+      // Supplying a new itemSize alone should not impact anything.
+      // Although the list get re-rendered by passing inline functions,
+      // but it still use the cached metrics to calculate the estimated size.
+      rendered.update(
+        <VariableSizeList
+          {...defaultProps}
+          estimatedItemSize={30}
+          overscanCount={1}
+          itemSize={index => 20}
+        />
+      );
+      expect(scrollContainer.props.style.height).toEqual(575);
+      // Reset calculation cache but don't re-render the list,
+      // the estimated total size should stay the same.
+      rendered.getInstance().resetAfterIndex(0, false);
+      expect(scrollContainer.props.style.height).toEqual(575);
+      // Pass inline function to make the list re-render.
+      rendered.update(
+        <VariableSizeList
+          {...defaultProps}
+          estimatedItemSize={30}
+          overscanCount={1}
+          itemSize={index => 20}
+        />
+      );
+      // The estimated total height should be (100 + 20 * 1 + 30 * 14)px = 540px.
+      expect(scrollContainer.props.style.height).toEqual(540);
+    });
+
     it('should re-render items after the specified index with updated styles', () => {
       const itemSize = jest.fn(() => 75);
       const rendered = ReactTestRenderer.create(

diff --git a/website/src/routes/api/VariableSizeGrid.js b/website/src/routes/api/VariableSizeGrid.js
@@ -96,39 +96,65 @@ const PROPS = [
 const METHODS = [
   {
     description: (
-      <p>
-        <code>VariableSizeGrid</code> caches offsets and measurements for each
-        column index for performance purposes. This method clears that cached
-        data for all columns after (and including) the specified index. It
-        should be called whenever a column's width changes. (Note that this is
-        not a typical occurrance.)
-      </p>
+      <Fragment>
+        <p>
+          <code>VariableSizeGrid</code> caches offsets and measurements for each
+          column index for performance purposes. This method clears that cached
+          data for all columns after (and including) the specified index. It
+          should be called whenever a column's width changes. (Note that this is
+          not a typical occurrance.)
+        </p>
+        <p>
+          By default the grid will automatically re-render after the index is
+          reset. If you would like to delay this re-render until e.g. a state
+          update has completed in the parent component, specify a value of
+          <code>false</code> for the second, optional parameter.
+        </p>
+      </Fragment>
     ),
-    signature: 'resetAfterColumnIndex(index: number): void',
+    signature:
+      'resetAfterColumnIndex(index: number, shouldForceUpdate: boolean = true): void',
   },
   {
     description: (
-      <p>
-        <code>VariableSizeGrid</code> caches offsets and measurements for each
-        item for performance purposes. This method clears that cached data for
-        all items after (and including) the specified indices. It should be
-        called whenever an items size changes. (Note that this is not a typical
-        occurrance.)
-      </p>
+      <Fragment>
+        <p>
+          <code>VariableSizeGrid</code> caches offsets and measurements for each
+          item for performance purposes. This method clears that cached data for
+          all items after (and including) the specified indices. It should be
+          called whenever an items size changes. (Note that this is not a
+          typical occurrance.)
+        </p>
+        <p>
+          By default the grid will automatically re-render after the index is
+          reset. If you would like to delay this re-render until e.g. a state
+          update has completed in the parent component, specify a value of
+          <code>false</code> for the optional <code>shouldForceUpdate</code> parameter.
+        </p>
+      </Fragment>
     ),
     signature:
-      'resetAfterIndices({ columnIndex: number, rowIndex: number }): void',
+      'resetAfterIndices({ columnIndex: number, rowIndex: number, shouldForceUpdate: boolean = true }): void',
   },
   {
     description: (
-      <p>
-        <code>VariableSizeGrid</code> caches offsets and measurements for each
-        row index for performance purposes. This method clears that cached data
-        for all rows after (and including) the specified index. It should be
-        called whenever a row's height changes. (Note that this is not a typical
-        occurrance.)
-      </p>
+      <Fragment>
+        <p>
+          <code>VariableSizeGrid</code> caches offsets and measurements for each
+          row index for performance purposes. This method clears that cached
+          data for all rows after (and including) the specified index. It should
+          be called whenever a row's height changes. (Note that this is not a
+          typical occurrance.)
+        </p>
+        <p>
+          By default the grid will automatically re-render after the index is
+          reset. If you would like to delay this re-render until e.g. a state
+          update has completed in the parent component, specify a value of
+          <code>false</code> for the second, optional parameter.
+        </p>
+      </Fragment>
     ),
-    signature: 'resetAfterRowIndex(index: number): void',
+    signature:
+      'resetAfterRowIndex(index: number, shouldForceUpdate: boolean = true): void',
   },
 ];
diff --git a/website/src/routes/api/VariableSizeList.js b/website/src/routes/api/VariableSizeList.js
@@ -74,14 +74,24 @@ const PROPS = [
 const METHODS = [
   {
     description: (
-      <p>
-        <code>VariableSizeList</code> caches offsets and measurements for each
-        index for performance purposes. This method clears that cached data for
-        all items after (and including) the specified index. It should be called
-        whenever a item's size changes. (Note that this is not a typical
-        occurrance.)
-      </p>
+      <Fragment>
+        <p>
+          <code>VariableSizeList</code> caches offsets and measurements for each
+          index for performance purposes. This method clears that cached data
+          for all items after (and including) the specified index. It should be
+          called whenever a item's size changes. (Note that this is not a
+          typical occurrance.)
+        </p>
+        <p>
+          By default the list will automatically re-render after the index is
+          reset. If you would like to delay this re-render until e.g. a state
+          update has completed in the parent component, specify a value of
+          <code>false</code>
+          for the second, optional parameter.
+        </p>
+      </Fragment>
     ),
-    signature: 'resetAfterIndex(index: number): void',
+    signature:
+      'resetAfterIndex(index: number, shouldForceUpdate: boolean = true): void',
   },
 ];