From 3559e42c55366bacd9bb5178ecab64f95e9a8ea7 Mon Sep 17 00:00:00 2001 From: Logan Daniels Date: Thu, 21 Dec 2017 18:13:04 -0800 Subject: [PATCH] Make sure VirtualizedList's windowSize is greater than 0 Summary: Setting `windowSize = 0` doesn't make sense. Let's make sure we catch this problem in the constructor so that it doesn't cause inexplicable list behavior. Also fixed an invariant in `VirtualizeUtils` that is meant to prohibit non-monotonically-increasing offset arrays. As written, the invariant condition can never actually be violated. Reviewed By: sahrens Differential Revision: D6625302 fbshipit-source-id: b2a983cbe7bb5fbe0aed7c5d59e69a8a00672993 --- Libraries/Lists/VirtualizeUtils.js | 8 +++++--- Libraries/Lists/VirtualizedList.js | 5 +++++ Libraries/Lists/__tests__/VirtualizeUtils-test.js | 11 +++++++++++ .../__snapshots__/VirtualizeUtils-test.js.snap | 3 +++ 4 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap diff --git a/Libraries/Lists/VirtualizeUtils.js b/Libraries/Lists/VirtualizeUtils.js index b665bd2d28ad8d..c43b13251eb3c3 100644 --- a/Libraries/Lists/VirtualizeUtils.js +++ b/Libraries/Lists/VirtualizeUtils.js @@ -25,17 +25,19 @@ function elementsThatOverlapOffsets( getFrameMetrics: (index: number) => {length: number, offset: number}, ): Array { const out = []; + let outLength = 0; for (let ii = 0; ii < itemCount; ii++) { const frame = getFrameMetrics(ii); const trailingOffset = frame.offset + frame.length; for (let kk = 0; kk < offsets.length; kk++) { if (out[kk] == null && trailingOffset >= offsets[kk]) { out[kk] = ii; + outLength++; if (kk === offsets.length - 1) { invariant( - out.length === offsets.length, - 'bad offsets input, should be in increasing order ' + - JSON.stringify(offsets), + outLength === offsets.length, + 'bad offsets input, should be in increasing order: %s', + JSON.stringify(offsets), ); return out; } diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 77c01bd6c1747f..83b31ee7a56a47 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -535,6 +535,11 @@ class VirtualizedList extends React.PureComponent { 'to support native onScroll events with useNativeDriver', ); + invariant( + props.windowSize > 0, + 'VirtualizedList: The windowSize prop must be present and set to a value greater than 0.', + ); + this._fillRateHelper = new FillRateHelper(this._getFrameMetrics); this._updateCellsToRenderBatcher = new Batchinator( this._updateCellsToRender, diff --git a/Libraries/Lists/__tests__/VirtualizeUtils-test.js b/Libraries/Lists/__tests__/VirtualizeUtils-test.js index 482574c12a4c2f..3e8b2c9433f592 100644 --- a/Libraries/Lists/__tests__/VirtualizeUtils-test.js +++ b/Libraries/Lists/__tests__/VirtualizeUtils-test.js @@ -81,4 +81,15 @@ describe('elementsThatOverlapOffsets', function() { elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii]), ).toEqual([1]); }); + it('errors on non-increasing offsets', function() { + const offsets = [150, 50]; + const frames = [ + {offset: 0, length: 50}, + {offset: 50, length: 150}, + {offset: 250, length: 100}, + ]; + expect(() => { + elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii]); + }).toThrowErrorMatchingSnapshot(); + }); }); diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap new file mode 100644 index 00000000000000..6daafe6961fd72 --- /dev/null +++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`elementsThatOverlapOffsets errors on non-increasing offsets 1`] = `"bad offsets input, should be in increasing order: [150,50]"`;