Skip to content

Commit d49cc52

Browse files
authored
fix(virtual-core): invalidate measurements when gap option changes (#1223)
1 parent 6cbecd8 commit d49cc52

4 files changed

Lines changed: 75 additions & 3 deletions

File tree

.changeset/eighty-ants-slide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/virtual-core': patch
3+
---
4+
5+
Made gap option changes invalidate measurements

packages/react-virtual/tests/index.test.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ interface ListProps {
2727
itemSize?: number
2828
rangeExtractor?: (range: Range) => number[]
2929
dynamic?: boolean
30+
gap?: number
3031
}
3132

3233
function List({
@@ -37,6 +38,7 @@ function List({
3738
itemSize,
3839
rangeExtractor,
3940
dynamic,
41+
gap,
4042
}: ListProps) {
4143
renderer()
4244

@@ -57,6 +59,7 @@ function List({
5759
},
5860
measureElement: () => itemSize ?? 0,
5961
rangeExtractor,
62+
gap,
6063
})
6164

6265
React.useEffect(() => {
@@ -160,6 +163,23 @@ test('should handle count change', () => {
160163
expect(screen.queryByText('Row 5')).not.toBeInTheDocument()
161164
})
162165

166+
test('should handle gap change', () => {
167+
const { rerender } = render(<List count={10} gap={0} />)
168+
169+
expect(screen.getByTestId('item-1')).toHaveStyle({
170+
transform: 'translateY(50px)',
171+
})
172+
173+
rerender(<List count={10} gap={40} />)
174+
175+
expect(screen.getByTestId('item-1')).toHaveStyle({
176+
transform: 'translateY(90px)',
177+
})
178+
expect(screen.getByTestId('item-2')).toHaveStyle({
179+
transform: 'translateY(180px)',
180+
})
181+
})
182+
163183
test('should handle handle height change', () => {
164184
const { rerender } = render(<List count={0} height={0} />)
165185

packages/virtual-core/src/index.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,7 @@ export class Virtualizer<
10911091
this.options.enabled,
10921092
this.options.lanes,
10931093
this.options.laneAssignmentMode,
1094+
this.options.gap,
10941095
],
10951096
(
10961097
count,
@@ -1100,6 +1101,7 @@ export class Virtualizer<
11001101
enabled,
11011102
lanes,
11021103
laneAssignmentMode,
1104+
gap,
11031105
) => {
11041106
const lanesChanged =
11051107
this.prevLanes !== undefined && this.prevLanes !== lanes
@@ -1120,6 +1122,7 @@ export class Virtualizer<
11201122
enabled,
11211123
lanes,
11221124
laneAssignmentMode,
1125+
gap,
11231126
}
11241127
},
11251128
{
@@ -1138,6 +1141,7 @@ export class Virtualizer<
11381141
enabled,
11391142
lanes,
11401143
laneAssignmentMode,
1144+
gap,
11411145
},
11421146
_itemSizeCacheVersion,
11431147
) => {
@@ -1196,7 +1200,6 @@ export class Virtualizer<
11961200
// At n=100k this drops cold-mount cost from ~2.5ms (eager object
11971201
// allocation) to roughly the cost of a single typed-array fill.
11981202
if (lanes === 1) {
1199-
const gap = this.options.gap
12001203
// Reuse flat backing if large enough; else grow (preserving data
12011204
// before `min` to mirror the slice-and-rebuild contract).
12021205
const need = count * 2
@@ -1276,7 +1279,7 @@ export class Virtualizer<
12761279
const prevInLane =
12771280
prevIndex !== undefined ? measurements[prevIndex] : undefined
12781281
start = prevInLane
1279-
? prevInLane.end + this.options.gap
1282+
? prevInLane.end + gap
12801283
: paddingStart + scrollMargin
12811284
} else if (filledLanes === lanes) {
12821285
// No cache, every lane seeded: place in the shortest lane.
@@ -1295,7 +1298,7 @@ export class Virtualizer<
12951298
}
12961299
}
12971300
lane = bestLane
1298-
start = bestEnd + this.options.gap
1301+
start = bestEnd + gap
12991302

13001303
if (shouldCacheLane) {
13011304
this.laneAssignments.set(i, lane)

packages/virtual-core/tests/index.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,50 @@ test('lazy fast path: respects paddingStart + scrollMargin + gap', () => {
12711271
expect(m[1]!.size).toBe(40)
12721272
})
12731273

1274+
test('changing gap invalidates cached measurements', () => {
1275+
const baseOptions = {
1276+
count: 5,
1277+
estimateSize: () => 50,
1278+
getScrollElement: () => null,
1279+
scrollToFn: vi.fn(),
1280+
observeElementRect: vi.fn(),
1281+
observeElementOffset: vi.fn(),
1282+
}
1283+
const v = new Virtualizer({ ...baseOptions, gap: 0 })
1284+
let m = v['getMeasurements']()
1285+
expect(m[1]!.start).toBe(50)
1286+
expect(v.getTotalSize()).toBe(250)
1287+
1288+
v.setOptions({ ...baseOptions, gap: 40 })
1289+
m = v['getMeasurements']()
1290+
// start(i) = i * (size + gap); previously stale starts survived a gap-only
1291+
// change because gap was missing from the measurement memo dependencies
1292+
expect(m[1]!.start).toBe(90)
1293+
expect(m[2]!.start).toBe(180)
1294+
expect(v.getTotalSize()).toBe(410)
1295+
})
1296+
1297+
test('changing gap invalidates cached measurements (multi-lane)', () => {
1298+
const baseOptions = {
1299+
count: 4,
1300+
lanes: 2,
1301+
estimateSize: () => 50,
1302+
getScrollElement: () => null,
1303+
scrollToFn: vi.fn(),
1304+
observeElementRect: vi.fn(),
1305+
observeElementOffset: vi.fn(),
1306+
}
1307+
const v = new Virtualizer({ ...baseOptions, gap: 0 })
1308+
let m = v['getMeasurements']()
1309+
expect(m[2]!.start).toBe(50)
1310+
1311+
v.setOptions({ ...baseOptions, gap: 40 })
1312+
m = v['getMeasurements']()
1313+
// second row in each lane starts at prevInLane.end + gap
1314+
expect(m[2]!.start).toBe(90)
1315+
expect(m[3]!.start).toBe(90)
1316+
})
1317+
12741318
test('lazy fast path: VirtualItem fields are correct', () => {
12751319
const v = new Virtualizer({
12761320
count: 3,

0 commit comments

Comments
 (0)