Skip to content

Commit 6d9274c

Browse files
authored
fix(virtual-core): correct lane assignments when lane count changes dynamically (#1095)
1 parent de8c12f commit 6d9274c

File tree

3 files changed

+125
-1
lines changed

3 files changed

+125
-1
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@tanstack/virtual-core': patch
3+
---
4+
5+
Fix: Correct lane assignments when lane count changes dynamically
6+
7+
Fixed a critical bug where changing the number of lanes dynamically would cause layout breakage with incorrect lane assignments. When the lane count changed (e.g., from 3 to 2 columns in a responsive masonry layout), some virtual items would retain their old lane numbers, causing out-of-bounds errors and broken layouts.
8+
9+
**Root Cause**: After clearing measurements cache on lane change, the virtualizer was incorrectly restoring data from `initialMeasurementsCache`, which contained stale lane assignments from the previous lane count.
10+
11+
**Fix**: Skip `initialMeasurementsCache` restoration during lane transitions by checking the `lanesSettling` flag. This ensures all measurements are recalculated with correct lane assignments for the new lane count.
12+
13+
**Before**:
14+
15+
```typescript
16+
// With lanes = 2
17+
virtualItems.forEach((item) => {
18+
columns[item.lane].push(item) // ❌ Error: item.lane could be 3
19+
})
20+
```
21+
22+
**After**:
23+
24+
```typescript
25+
// With lanes = 2
26+
virtualItems.forEach((item) => {
27+
columns[item.lane].push(item) // ✅ item.lane is always 0 or 1
28+
})
29+
```
30+
31+
This fix is essential for responsive masonry layouts where column count changes based on viewport width. No performance impact as it only affects the lane change transition path.

packages/virtual-core/src/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,9 @@ export class Virtualizer<
687687
this.pendingMeasuredCacheIndexes = []
688688
}
689689

690-
if (this.measurementsCache.length === 0) {
690+
// Don't restore from initialMeasurementsCache during lane changes
691+
// as it contains stale lane assignments from the previous lane count
692+
if (this.measurementsCache.length === 0 && !this.lanesSettling) {
691693
this.measurementsCache = this.options.initialMeasurementsCache
692694
this.measurementsCache.forEach((item) => {
693695
this.itemSizeCache.set(item.key, item.size)

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,94 @@ test('should return correct total size with one item and multiple lanes', () =>
2929
})
3030
expect(virtualizer.getTotalSize()).toBe(50)
3131
})
32+
33+
test('should correctly recalculate lane assignments when lane count changes', () => {
34+
// Create a mock scroll element
35+
const mockScrollElement = {
36+
scrollTop: 0,
37+
scrollLeft: 0,
38+
offsetWidth: 400,
39+
offsetHeight: 600,
40+
} as unknown as HTMLDivElement
41+
42+
// Mock ResizeObserver
43+
let resizeCallback: ((entries: any[]) => void) | null = null
44+
const mockResizeObserver = vi.fn((callback) => {
45+
resizeCallback = callback
46+
return {
47+
observe: vi.fn(),
48+
unobserve: vi.fn(),
49+
disconnect: vi.fn(),
50+
}
51+
})
52+
global.ResizeObserver = mockResizeObserver as any
53+
54+
// Create virtualizer with 3 lanes initially
55+
const virtualizer = new Virtualizer({
56+
count: 10,
57+
lanes: 3,
58+
estimateSize: () => 100,
59+
getScrollElement: () => mockScrollElement,
60+
scrollToFn: vi.fn(),
61+
observeElementRect: (instance, cb) => {
62+
cb({ width: 400, height: 600 })
63+
return () => {}
64+
},
65+
observeElementOffset: (instance, cb) => {
66+
cb(0, false)
67+
return () => {}
68+
},
69+
})
70+
71+
virtualizer._willUpdate()
72+
73+
// Get initial measurements with 3 lanes
74+
let measurements = virtualizer['getMeasurements']()
75+
expect(measurements.length).toBe(10)
76+
77+
// All lane assignments should be 0, 1, or 2 (3 lanes)
78+
measurements.forEach((item) => {
79+
expect(item.lane).toBeGreaterThanOrEqual(0)
80+
expect(item.lane).toBeLessThan(3)
81+
})
82+
83+
// Change to 2 lanes
84+
virtualizer.setOptions({
85+
count: 10,
86+
lanes: 2,
87+
estimateSize: () => 100,
88+
getScrollElement: () => mockScrollElement,
89+
scrollToFn: vi.fn(),
90+
observeElementRect: (instance, cb) => {
91+
cb({ width: 400, height: 600 })
92+
return () => {}
93+
},
94+
observeElementOffset: (instance, cb) => {
95+
cb(0, false)
96+
return () => {}
97+
},
98+
})
99+
100+
virtualizer._willUpdate()
101+
102+
// Get new measurements with 2 lanes
103+
measurements = virtualizer['getMeasurements']()
104+
expect(measurements.length).toBe(10)
105+
106+
// All lane assignments should now be 0 or 1 (2 lanes)
107+
// This is the bug fix - previously some items could still have lane: 2
108+
measurements.forEach((item, index) => {
109+
expect(item.lane).toBeGreaterThanOrEqual(0)
110+
expect(item.lane).toBeLessThan(2)
111+
})
112+
113+
// Verify no out of bounds access would occur
114+
const lanes = 2
115+
const columns = Array.from({ length: lanes }, () => [] as typeof measurements)
116+
measurements.forEach((item) => {
117+
// This should not throw
118+
expect(() => {
119+
columns[item.lane].push(item)
120+
}).not.toThrow()
121+
})
122+
})

0 commit comments

Comments
 (0)