diff --git a/CHANGELOG.md b/CHANGELOG.md
index e49c882..b7dac11 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,9 @@
Changelog
------------
+### 1.7.2
+* 🐛 Add guards to avoid invalid scroll offsets when `scrollTo()` is called with a negative offset or when `scrollToItem` is called with invalid indices (negative or too large).
+
### 1.7.1
* 🐛 Fix SSR regression introduced in 1.7.0 - ([Betree](https://github.com/Betree) - [#185](https://github.com/bvaughn/react-window/pull/185))
diff --git a/package.json b/package.json
index 6c6d90d..5b030bb 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "react-window",
- "version": "1.7.1",
+ "version": "1.7.2",
"description":
"React components for efficiently rendering large, scrollable lists and tabular data",
"author":
diff --git a/src/__tests__/FixedSizeGrid.js b/src/__tests__/FixedSizeGrid.js
index 3a2325f..032b27f 100644
--- a/src/__tests__/FixedSizeGrid.js
+++ b/src/__tests__/FixedSizeGrid.js
@@ -431,31 +431,30 @@ describe('FixedSizeGrid', () => {
})
);
});
+
+ it('should ignore offsets less than zero', () => {
+ const onScroll = jest.fn();
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ instance.scrollTo({ scrollLeft: 100, scrollTop: 100 });
+ onScroll.mockClear();
+ instance.scrollTo({ scrollLeft: -1, scrollTop: -1 });
+ expect(onScroll.mock.calls[0][0].scrollLeft).toBe(0);
+ expect(onScroll.mock.calls[0][0].scrollTop).toBe(0);
+ });
});
describe('scrollToItem method', () => {
it('should not set invalid offsets when the list contains few items', () => {
- const onScroll = jest.fn();
const rendered = ReactTestRenderer.create(
-
+
);
- onScroll.mockClear();
- // Offset should not be negative.
- rendered
- .getInstance()
- .scrollToItem({ columnIndex: 0, rowIndex: 0, align: 'auto' });
- expect(onScroll).toHaveBeenCalledWith({
- horizontalScrollDirection: 'backward',
- scrollLeft: 0,
- scrollTop: 0,
- scrollUpdateWasRequested: true,
- verticalScrollDirection: 'backward',
- });
+ expect(onItemsRendered).toMatchSnapshot();
+ onItemsRendered.mockClear();
+ rendered.getInstance().scrollToItem(0);
+ expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -679,6 +678,30 @@ describe('FixedSizeGrid', () => {
verticalScrollDirection: 'backward',
});
});
+
+ it('should ignore indexes less than zero', () => {
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ instance.scrollToItem({ columnIndex: 20, rowIndex: 20 });
+ onItemsRendered.mockClear();
+ instance.scrollToItem({ columnIndex: -1, rowIndex: -1 });
+ expect(onItemsRendered.mock.calls).toMatchSnapshot();
+ });
+
+ it('should ignore indexes greater than itemCount', () => {
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ onItemsRendered.mockClear();
+ instance.scrollToItem({
+ columnIndex: defaultProps.columnCount * 2,
+ rowIndex: defaultProps.rowCount * 2,
+ });
+ expect(onItemsRendered.mock.calls).toMatchSnapshot();
+ });
});
// onItemsRendered is pretty well covered by other snapshot tests
diff --git a/src/__tests__/FixedSizeList.js b/src/__tests__/FixedSizeList.js
index f591c3d..a472018 100644
--- a/src/__tests__/FixedSizeList.js
+++ b/src/__tests__/FixedSizeList.js
@@ -363,6 +363,18 @@ describe('FixedSizeList', () => {
instance.scrollTo(100);
expect(itemRenderer.mock.calls[0][0].isScrolling).toBe(false);
});
+
+ it('should ignore values less than zero', () => {
+ const onScroll = jest.fn();
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ instance.scrollTo(100);
+ onScroll.mockClear();
+ instance.scrollTo(-1);
+ expect(onScroll.mock.calls[0][0].scrollOffset).toBe(0);
+ });
});
describe('scrollToItem method', () => {
@@ -371,14 +383,10 @@ describe('FixedSizeList', () => {
const rendered = ReactTestRenderer.create(
);
- onScroll.mockClear();
- // Offset should not be negative.
+ expect(onItemsRendered).toMatchSnapshot();
+ onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
- expect(onScroll).toHaveBeenCalledWith({
- scrollDirection: 'backward',
- scrollOffset: 0,
- scrollUpdateWasRequested: true,
- });
+ expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -457,6 +465,27 @@ describe('FixedSizeList', () => {
instance.scrollToItem(15);
expect(itemRenderer.mock.calls[0][0].isScrolling).toBe(false);
});
+
+ it('should ignore indexes less than zero', () => {
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ instance.scrollToItem(20);
+ onItemsRendered.mockClear();
+ instance.scrollToItem(-1);
+ expect(onItemsRendered.mock.calls).toMatchSnapshot();
+ });
+
+ it('should ignore indexes greater than itemCount', () => {
+ const instance = ReactDOM.render(
+ ,
+ document.createElement('div')
+ );
+ onItemsRendered.mockClear();
+ instance.scrollToItem(defaultProps.itemCount * 2);
+ expect(onItemsRendered.mock.calls).toMatchSnapshot();
+ });
});
// onItemsRendered is pretty well covered by other snapshot tests
diff --git a/src/__tests__/VariableSizeGrid.js b/src/__tests__/VariableSizeGrid.js
index 645e589..9ae387b 100644
--- a/src/__tests__/VariableSizeGrid.js
+++ b/src/__tests__/VariableSizeGrid.js
@@ -159,18 +159,10 @@ describe('VariableSizeGrid', () => {
onScroll={onScroll}
/>
);
- onScroll.mockClear();
- // Offset should not be negative.
- rendered
- .getInstance()
- .scrollToItem({ columnIndex: 0, rowIndex: 0, align: 'auto' });
- expect(onScroll).toHaveBeenCalledWith({
- horizontalScrollDirection: 'backward',
- scrollLeft: 0,
- scrollTop: 0,
- scrollUpdateWasRequested: true,
- verticalScrollDirection: 'backward',
- });
+ expect(onItemsRendered).toMatchSnapshot();
+ onItemsRendered.mockClear();
+ rendered.getInstance().scrollToItem(0);
+ expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
@@ -290,12 +282,12 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
- .scrollToItem({ columnIndex: 15, rowIndex: 10, align: 'end' });
+ .scrollToItem({ columnIndex: 5, rowIndex: 10, align: 'end' });
// With hidden scrollbars (size === 0) we would expect...
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
- scrollLeft: 720,
+ scrollLeft: 115,
scrollTop: 230,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'forward',
@@ -306,12 +298,12 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
- .scrollToItem({ columnIndex: 15, rowIndex: 10, align: 'end' });
+ .scrollToItem({ columnIndex: 5, rowIndex: 10, align: 'end' });
// With scrollbars of size 20 we would expect those values ot increase by 20px
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
- scrollLeft: 740,
+ scrollLeft: 135,
scrollTop: 250,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'forward',
@@ -352,13 +344,13 @@ describe('VariableSizeGrid', () => {
onScroll.mockClear();
rendered
.getInstance()
- .scrollToItem({ columnIndex: 15, rowIndex: 0, align: 'end' });
+ .scrollToItem({ columnIndex: 5, rowIndex: 0, align: 'end' });
// Since there aren't enough rows to require vertical scrolling,
// the additional 20px for the scrollbar should not be taken into consideration.
expect(onScroll).toHaveBeenCalledWith({
horizontalScrollDirection: 'forward',
- scrollLeft: 720,
+ scrollLeft: 115,
scrollTop: 0,
scrollUpdateWasRequested: true,
verticalScrollDirection: 'backward',
diff --git a/src/__tests__/VariableSizeList.js b/src/__tests__/VariableSizeList.js
index ccfcf59..d9c0ffb 100644
--- a/src/__tests__/VariableSizeList.js
+++ b/src/__tests__/VariableSizeList.js
@@ -121,14 +121,10 @@ describe('VariableSizeList', () => {
const rendered = ReactTestRenderer.create(
);
- onScroll.mockClear();
- // Offset should not be negative.
+ expect(onItemsRendered).toMatchSnapshot();
+ onItemsRendered.mockClear();
rendered.getInstance().scrollToItem(0);
- expect(onScroll).toHaveBeenCalledWith({
- scrollDirection: 'backward',
- scrollOffset: 0,
- scrollUpdateWasRequested: true,
- });
+ expect(onItemsRendered).not.toHaveBeenCalled();
});
it('should scroll to the correct item for align = "auto"', () => {
diff --git a/src/__tests__/__snapshots__/FixedSizeGrid.js.snap b/src/__tests__/__snapshots__/FixedSizeGrid.js.snap
index 356b369..5bd209e 100644
--- a/src/__tests__/__snapshots__/FixedSizeGrid.js.snap
+++ b/src/__tests__/__snapshots__/FixedSizeGrid.js.snap
@@ -273,6 +273,42 @@ Array [
]
`;
+exports[`FixedSizeGrid scrollToItem method should ignore indexes greater than itemCount 1`] = `
+Array [
+ Array [
+ Object {
+ "overscanColumnStartIndex": 97,
+ "overscanColumnStopIndex": 99,
+ "overscanRowStartIndex": 95,
+ "overscanRowStopIndex": 99,
+ "visibleColumnStartIndex": 98,
+ "visibleColumnStopIndex": 99,
+ "visibleRowStartIndex": 96,
+ "visibleRowStopIndex": 99,
+ },
+ ],
+]
+`;
+
+exports[`FixedSizeGrid scrollToItem method should ignore indexes less than zero 1`] = `
+Array [
+ Array [
+ Object {
+ "overscanColumnStartIndex": 0,
+ "overscanColumnStopIndex": 3,
+ "overscanRowStartIndex": 0,
+ "overscanRowStopIndex": 5,
+ "visibleColumnStartIndex": 0,
+ "visibleColumnStopIndex": 2,
+ "visibleRowStartIndex": 0,
+ "visibleRowStopIndex": 4,
+ },
+ ],
+]
+`;
+
+exports[`FixedSizeGrid scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
+
exports[`FixedSizeGrid scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
diff --git a/src/__tests__/__snapshots__/FixedSizeList.js.snap b/src/__tests__/__snapshots__/FixedSizeList.js.snap
index ea1e024..e62b525 100644
--- a/src/__tests__/__snapshots__/FixedSizeList.js.snap
+++ b/src/__tests__/__snapshots__/FixedSizeList.js.snap
@@ -153,6 +153,34 @@ Array [
]
`;
+exports[`FixedSizeList scrollToItem method should ignore indexes greater than itemCount 1`] = `
+Array [
+ Array [
+ Object {
+ "overscanStartIndex": 94,
+ "overscanStopIndex": 99,
+ "visibleStartIndex": 96,
+ "visibleStopIndex": 99,
+ },
+ ],
+]
+`;
+
+exports[`FixedSizeList scrollToItem method should ignore indexes less than zero 1`] = `
+Array [
+ Array [
+ Object {
+ "overscanStartIndex": 0,
+ "overscanStopIndex": 6,
+ "visibleStartIndex": 0,
+ "visibleStopIndex": 4,
+ },
+ ],
+]
+`;
+
+exports[`FixedSizeList scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
+
exports[`FixedSizeList scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
diff --git a/src/__tests__/__snapshots__/VariableSizeGrid.js.snap b/src/__tests__/__snapshots__/VariableSizeGrid.js.snap
index 407f806..ecf8cdf 100644
--- a/src/__tests__/__snapshots__/VariableSizeGrid.js.snap
+++ b/src/__tests__/__snapshots__/VariableSizeGrid.js.snap
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
+exports[`VariableSizeGrid scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
+
exports[`VariableSizeGrid scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
diff --git a/src/__tests__/__snapshots__/VariableSizeList.js.snap b/src/__tests__/__snapshots__/VariableSizeList.js.snap
index fdfea7c..c0ef719 100644
--- a/src/__tests__/__snapshots__/VariableSizeList.js.snap
+++ b/src/__tests__/__snapshots__/VariableSizeList.js.snap
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
+exports[`VariableSizeList scrollToItem method should not set invalid offsets when the list contains few items 1`] = `[Function]`;
+
exports[`VariableSizeList scrollToItem method should scroll to the correct item for align = "auto" 1`] = `
Array [
Array [
diff --git a/src/createGridComponent.js b/src/createGridComponent.js
index 55a2983..c5b125b 100644
--- a/src/createGridComponent.js
+++ b/src/createGridComponent.js
@@ -221,6 +221,13 @@ export default function createGridComponent({
scrollLeft: number,
scrollTop: number,
}): void {
+ if (scrollLeft !== undefined) {
+ scrollLeft = Math.max(0, scrollLeft);
+ }
+ if (scrollTop !== undefined) {
+ scrollTop = Math.max(0, scrollTop);
+ }
+
this.setState(prevState => {
if (scrollLeft === undefined) {
scrollLeft = prevState.scrollLeft;
@@ -229,6 +236,13 @@ export default function createGridComponent({
scrollTop = prevState.scrollTop;
}
+ if (
+ prevState.scrollLeft === scrollLeft &&
+ prevState.scrollTop === scrollTop
+ ) {
+ return null;
+ }
+
return {
horizontalScrollDirection:
prevState.scrollLeft < scrollLeft ? 'forward' : 'backward',
@@ -250,10 +264,17 @@ export default function createGridComponent({
columnIndex?: number,
rowIndex?: number,
}): void {
- const { height, width } = this.props;
+ const { columnCount, height, rowCount, width } = this.props;
const { scrollLeft, scrollTop } = this.state;
const scrollbarSize = getScrollbarSize();
+ if (columnIndex !== undefined) {
+ columnIndex = Math.max(0, Math.min(columnIndex, columnCount - 1));
+ }
+ if (rowIndex !== undefined) {
+ rowIndex = Math.max(0, Math.min(rowIndex, rowCount - 1));
+ }
+
const estimatedTotalHeight = getEstimatedTotalHeight(
this.props,
this._instanceProps
@@ -677,7 +698,7 @@ export default function createGridComponent({
const { direction } = this.props;
// HACK According to the spec, scrollLeft should be negative for RTL aligned elements.
- // Chrome does not seem to adhere; its scrolLeft values are positive (measured relative to the left).
+ // Chrome does not seem to adhere; its scrollLeft values are positive (measured relative to the left).
// See https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollLeft
let calculatedScrollLeft = scrollLeft;
if (direction === 'rtl') {
diff --git a/src/createListComponent.js b/src/createListComponent.js
index 488fdb4..afef79c 100644
--- a/src/createListComponent.js
+++ b/src/createListComponent.js
@@ -180,19 +180,27 @@ export default function createListComponent({
}
scrollTo(scrollOffset: number): void {
- this.setState(
- prevState => ({
+ scrollOffset = Math.max(0, scrollOffset);
+
+ this.setState(prevState => {
+ if (prevState.scrollOffset === scrollOffset) {
+ return null;
+ }
+ return {
scrollDirection:
prevState.scrollOffset < scrollOffset ? 'forward' : 'backward',
scrollOffset: scrollOffset,
scrollUpdateWasRequested: true,
- }),
- this._resetIsScrollingDebounced
- );
+ };
+ }, this._resetIsScrollingDebounced);
}
scrollToItem(index: number, align: ScrollToAlign = 'auto'): void {
+ const { itemCount } = this.props;
const { scrollOffset } = this.state;
+
+ index = Math.max(0, Math.min(index, itemCount - 1));
+
this.scrollTo(
getOffsetForIndexAndAlignment(
this.props,