Skip to content

Commit c0dcc15

Browse files
committed
Relax List rowHeight constraint
1 parent 4361ba6 commit c0dcc15

25 files changed

+1067
-77
lines changed

lib/components/grid/Grid.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ export function Grid<
221221
rowIndex++
222222
) {
223223
const rowBounds = cachedRowBounds.getItemBounds(rowIndex);
224-
const rowOffset = rowBounds?.scrollOffset;
224+
const rowOffset = rowBounds?.scrollOffset ?? 0;
225225

226226
const columns: ReactNode[] = [];
227227

@@ -231,7 +231,7 @@ export function Grid<
231231
columnIndex++
232232
) {
233233
const columnBounds = cachedColumnBounds.getItemBounds(columnIndex);
234-
const columnOffset = columnBounds?.scrollOffset;
234+
const columnOffset = columnBounds?.scrollOffset ?? 0;
235235

236236
columns.push(
237237
<CellComponent

lib/components/list/List.test.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
disableResizeObserverForCurrentTest,
88
setDefaultElementSize,
99
setElementSize,
10+
setElementSizeFunction,
1011
simulateUnsupportedEnvironmentForTest
1112
} from "../../utils/test/mockResizeObserver";
1213
import { List } from "./List";
@@ -555,6 +556,30 @@ describe("List", () => {
555556

556557
expect(container.querySelectorAll('[role="listitem"]')).toHaveLength(4);
557558
});
559+
560+
// Most dynamic size tests are in useVirtualizer
561+
test("type: dynamic (lazily measured)", () => {
562+
setElementSizeFunction((element) => {
563+
const attribute = element.getAttribute("data-react-window-index");
564+
if (attribute !== null) {
565+
const index = parseInt(attribute);
566+
if (!Number.isNaN(index)) {
567+
return new DOMRect(0, 0, 100, (index + 1) * 5);
568+
}
569+
}
570+
});
571+
572+
const { container } = render(
573+
<List
574+
overscanCount={0}
575+
rowCount={50}
576+
rowComponent={RowComponent}
577+
rowProps={EMPTY_OBJECT}
578+
/>
579+
);
580+
581+
expect(container.querySelectorAll('[role="listitem"]')).toHaveLength(6);
582+
});
558583
});
559584

560585
describe("edge cases", () => {

lib/components/list/List.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export function List<
119119

120120
const offset =
121121
startIndexOverscan >= 0
122-
? cachedBounds.getItemBounds(startIndexOverscan).scrollOffset
122+
? (cachedBounds.getItemBounds(startIndexOverscan)?.scrollOffset ?? 0)
123123
: 0;
124124

125125
const rows = useMemo(() => {
@@ -131,10 +131,18 @@ export function List<
131131
index++
132132
) {
133133
const bounds = cachedBounds.getItemBounds(index);
134-
const style: CSSProperties = {
135-
height: hasRowHeight ? bounds.size : undefined,
136-
width: "100%"
137-
};
134+
135+
let style: CSSProperties = {};
136+
if (bounds) {
137+
style = {
138+
height: hasRowHeight ? bounds.size : undefined,
139+
width: "100%"
140+
};
141+
} else {
142+
style = {
143+
width: "100%"
144+
};
145+
}
138146

139147
if (index === startIndexOverscan) {
140148
style.marginTop = `${offset}px`;

lib/components/list/types.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,15 @@ export type ListProps<
9595
* - number of pixels (number)
9696
* - percentage of the grid's current height (string)
9797
* - function that returns the row height (in pixels) given an index and `cellProps`
98+
*
99+
* If this property is not provided, a ResizeObserver will be used to measure the height of a row's content.
100+
*
101+
* ⚠️ Row height should be provided if it is known ahead of time as that is the most efficient way to render the list.
98102
*/
99-
rowHeight: number | string | ((index: number, cellProps: RowProps) => number);
103+
rowHeight?:
104+
| number
105+
| string
106+
| ((index: number, cellProps: RowProps) => number);
100107

101108
/**
102109
* Additional props to be passed to the row-rendering component.

lib/core/createCachedBounds.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,62 @@ describe("createCachedBounds", () => {
6868
cachedBounds.getItemBounds(1);
6969
}).toThrow("Invalid index 1");
7070
});
71+
72+
test("should gracefully handle undefined item sizes", () => {
73+
const cachedBounds = createCachedBounds({
74+
itemCount: 10,
75+
itemProps: {},
76+
itemSize: undefined
77+
});
78+
79+
cachedBounds.setItemSize(0, 10);
80+
81+
expect(cachedBounds.size).toBe(1);
82+
expect(cachedBounds.getItemBounds(0)).toEqual({
83+
scrollOffset: 0,
84+
size: 10
85+
});
86+
87+
cachedBounds.setItemSize(1, 20);
88+
89+
expect(cachedBounds.size).toBe(2);
90+
expect(cachedBounds.getItemBounds(1)).toEqual({
91+
scrollOffset: 10,
92+
size: 20
93+
});
94+
});
95+
96+
test("should gracefully handle sparsely populated cache", () => {
97+
const cachedBounds = createCachedBounds({
98+
itemCount: 5,
99+
itemProps: {},
100+
itemSize: undefined
101+
});
102+
103+
expect(cachedBounds.getEstimatedSize()).toBeUndefined();
104+
105+
// Estimated average should be based on measured cells
106+
cachedBounds.setItemSize(0, 10);
107+
expect(cachedBounds.getEstimatedSize()).toBe(10);
108+
cachedBounds.setItemSize(2, 20);
109+
expect(cachedBounds.getEstimatedSize()).toBe(15);
110+
cachedBounds.setItemSize(4, 30);
111+
expect(cachedBounds.getEstimatedSize()).toBe(20);
112+
113+
// Bounds offsets based on measured cells; gaps should be filled in by averages
114+
expect(cachedBounds.getItemBounds(0)).toEqual({
115+
scrollOffset: 0,
116+
size: 10
117+
});
118+
expect(cachedBounds.getItemBounds(1)).toBeUndefined();
119+
expect(cachedBounds.getItemBounds(2)).toEqual({
120+
scrollOffset: 30,
121+
size: 20
122+
});
123+
expect(cachedBounds.getItemBounds(3)).toBeUndefined();
124+
expect(cachedBounds.getItemBounds(4)).toEqual({
125+
scrollOffset: 70,
126+
size: 30
127+
});
128+
});
71129
});

lib/core/createCachedBounds.ts

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { assert } from "../utils/assert";
2-
import type { Bounds, CachedBounds, SizeFunction } from "./types";
2+
import type { CachedBounds, SizeFunction } from "./types";
33

44
export function createCachedBounds<Props extends object>({
55
itemCount,
@@ -8,66 +8,89 @@ export function createCachedBounds<Props extends object>({
88
}: {
99
itemCount: number;
1010
itemProps: Props;
11-
itemSize: number | SizeFunction<Props>;
11+
itemSize: number | SizeFunction<Props> | undefined;
1212
}): CachedBounds {
13-
const cache = new Map<number, Bounds>();
13+
const cache = new Map<number, number>();
14+
15+
let lastContiguousMeasuredIndex = -1;
16+
let totalSize = 0;
1417

1518
const api = {
1619
getEstimatedSize() {
1720
if (itemCount === 0) {
1821
return 0;
1922
} else {
20-
const bounds = api.getItemBounds(cache.size === 0 ? 0 : cache.size - 1);
21-
assert(bounds, "Unexpected bounds cache miss");
23+
if (cache.size === 0 && itemSize !== undefined) {
24+
// If possible, lazily measure the first row so we can estimate a size
25+
api.getItemBounds(0);
26+
}
2227

23-
return (bounds.scrollOffset + bounds.size) / cache.size;
28+
return cache.size === 0 ? undefined : totalSize / cache.size;
2429
}
2530
},
2631
getItemBounds(index: number) {
2732
assert(index < itemCount, `Invalid index ${index}`);
2833

29-
while (cache.size - 1 < index) {
30-
const currentIndex = cache.size;
34+
let size: number | undefined = undefined;
3135

32-
let size: number = 0;
33-
switch (typeof itemSize) {
34-
case "function": {
35-
size = itemSize(currentIndex, itemProps);
36-
break;
37-
}
38-
case "number": {
39-
size = itemSize;
40-
break;
36+
if (itemSize) {
37+
while (cache.size <= index) {
38+
const currentIndex = cache.size;
39+
40+
switch (typeof itemSize) {
41+
case "function": {
42+
size = itemSize(currentIndex, itemProps);
43+
break;
44+
}
45+
case "number": {
46+
size = itemSize;
47+
break;
48+
}
4149
}
50+
51+
lastContiguousMeasuredIndex = currentIndex;
52+
totalSize += size;
53+
54+
cache.set(currentIndex, size);
4255
}
4356

44-
if (currentIndex === 0) {
45-
cache.set(currentIndex, {
46-
scrollOffset: 0,
47-
size
48-
});
49-
} else {
50-
const previousRowBounds = cache.get(currentIndex - 1);
51-
assert(
52-
previousRowBounds !== undefined,
53-
`Unexpected bounds cache miss for index ${index}`
54-
);
55-
56-
cache.set(currentIndex, {
57-
scrollOffset:
58-
previousRowBounds.scrollOffset + previousRowBounds.size,
59-
size
60-
});
57+
size = cache.get(index);
58+
} else {
59+
size = cache.get(index);
60+
}
61+
62+
if (size !== undefined) {
63+
const averageSize = api.getEstimatedSize() ?? 0;
64+
65+
let scrollOffset = 0;
66+
67+
while (index > 0) {
68+
index--;
69+
scrollOffset += cache.get(index) ?? averageSize;
6170
}
71+
72+
return {
73+
scrollOffset,
74+
size
75+
};
76+
}
77+
},
78+
hasItemBounds(index: number) {
79+
return cache.has(index);
80+
},
81+
setItemSize(index: number, size: number) {
82+
const prevSize = cache.get(index);
83+
if (prevSize !== undefined) {
84+
totalSize -= prevSize;
6285
}
6386

64-
const bounds = cache.get(index);
65-
assert(
66-
bounds !== undefined,
67-
`Unexpected bounds cache miss for index ${index}`
68-
);
87+
totalSize += size;
88+
89+
if (index === lastContiguousMeasuredIndex + 1) {
90+
lastContiguousMeasuredIndex = index;
91+
}
6992

70-
return bounds;
93+
cache.set(index, size);
7194
},
7295
get size() {
7396
return cache.size;

lib/core/getEstimatedSize.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,48 @@ describe("getEstimatedSize", () => {
100100
).toBe(250);
101101
});
102102
});
103+
104+
describe("itemSize: undefined (lazily measured)", () => {
105+
test("should return undefined if no measurements have been taken", () => {
106+
expect(
107+
getEstimatedSize({
108+
cachedBounds: createCachedBounds({
109+
itemCount: 10,
110+
itemProps: EMPTY_OBJECT,
111+
itemSize: undefined
112+
}),
113+
itemCount: 10,
114+
itemSize: undefined
115+
})
116+
).toBeUndefined();
117+
});
118+
119+
test("should return an estimated size based on measurements that have been taken", () => {
120+
const cachedBounds = createCachedBounds({
121+
itemCount: 10,
122+
itemProps: EMPTY_OBJECT,
123+
itemSize: undefined
124+
});
125+
cachedBounds.setItemSize(0, 10);
126+
cachedBounds.setItemSize(1, 20);
127+
128+
expect(
129+
getEstimatedSize({
130+
cachedBounds,
131+
itemCount: 10,
132+
itemSize: undefined
133+
})
134+
).toBe(150);
135+
136+
cachedBounds.setItemSize(2, 30);
137+
138+
expect(
139+
getEstimatedSize({
140+
cachedBounds,
141+
itemCount: 10,
142+
itemSize: undefined
143+
})
144+
).toBe(200);
145+
});
146+
});
103147
});

lib/core/getEstimatedSize.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { CachedBounds, SizeFunction } from "./types";
2+
import { assert } from "../utils/assert";
23

34
export function getEstimatedSize<Props extends object>({
45
cachedBounds,
@@ -7,7 +8,7 @@ export function getEstimatedSize<Props extends object>({
78
}: {
89
cachedBounds: CachedBounds;
910
itemCount: number;
10-
itemSize: number | SizeFunction<Props>;
11+
itemSize: number | SizeFunction<Props> | undefined;
1112
}) {
1213
if (itemCount === 0) {
1314
return 0;
@@ -17,6 +18,7 @@ export function getEstimatedSize<Props extends object>({
1718
const bounds = cachedBounds.getItemBounds(
1819
cachedBounds.size === 0 ? 0 : cachedBounds.size - 1
1920
);
21+
assert(bounds !== undefined, "Unexpected bounds cache miss");
2022

2123
const averageItemSize =
2224
(bounds.scrollOffset + bounds.size) / cachedBounds.size;
@@ -26,6 +28,12 @@ export function getEstimatedSize<Props extends object>({
2628
case "number": {
2729
return itemCount * itemSize;
2830
}
31+
default: {
32+
const estimatedSize = cachedBounds.getEstimatedSize();
33+
if (estimatedSize !== undefined) {
34+
return estimatedSize * itemCount;
35+
}
36+
}
2937
}
3038
}
3139
}

0 commit comments

Comments
 (0)