diff --git a/.changeset/fix-lane-change-layout.md b/.changeset/fix-lane-change-layout.md new file mode 100644 index 00000000..3298bb9e --- /dev/null +++ b/.changeset/fix-lane-change-layout.md @@ -0,0 +1,31 @@ +--- +'@tanstack/virtual-core': patch +--- + +Fix: Correct lane assignments when lane count changes dynamically + +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. + +**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. + +**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. + +**Before**: + +```typescript +// With lanes = 2 +virtualItems.forEach((item) => { + columns[item.lane].push(item) // ❌ Error: item.lane could be 3 +}) +``` + +**After**: + +```typescript +// With lanes = 2 +virtualItems.forEach((item) => { + columns[item.lane].push(item) // ✅ item.lane is always 0 or 1 +}) +``` + +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. diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index bd646dec..34c51c5b 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -687,7 +687,9 @@ export class Virtualizer< this.pendingMeasuredCacheIndexes = [] } - if (this.measurementsCache.length === 0) { + // Don't restore from initialMeasurementsCache during lane changes + // as it contains stale lane assignments from the previous lane count + if (this.measurementsCache.length === 0 && !this.lanesSettling) { this.measurementsCache = this.options.initialMeasurementsCache this.measurementsCache.forEach((item) => { this.itemSizeCache.set(item.key, item.size) diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 12b49ffd..624fb805 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -29,3 +29,94 @@ test('should return correct total size with one item and multiple lanes', () => }) expect(virtualizer.getTotalSize()).toBe(50) }) + +test('should correctly recalculate lane assignments when lane count changes', () => { + // Create a mock scroll element + const mockScrollElement = { + scrollTop: 0, + scrollLeft: 0, + offsetWidth: 400, + offsetHeight: 600, + } as unknown as HTMLDivElement + + // Mock ResizeObserver + let resizeCallback: ((entries: any[]) => void) | null = null + const mockResizeObserver = vi.fn((callback) => { + resizeCallback = callback + return { + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + } + }) + global.ResizeObserver = mockResizeObserver as any + + // Create virtualizer with 3 lanes initially + const virtualizer = new Virtualizer({ + count: 10, + lanes: 3, + estimateSize: () => 100, + getScrollElement: () => mockScrollElement, + scrollToFn: vi.fn(), + observeElementRect: (instance, cb) => { + cb({ width: 400, height: 600 }) + return () => {} + }, + observeElementOffset: (instance, cb) => { + cb(0, false) + return () => {} + }, + }) + + virtualizer._willUpdate() + + // Get initial measurements with 3 lanes + let measurements = virtualizer['getMeasurements']() + expect(measurements.length).toBe(10) + + // All lane assignments should be 0, 1, or 2 (3 lanes) + measurements.forEach((item) => { + expect(item.lane).toBeGreaterThanOrEqual(0) + expect(item.lane).toBeLessThan(3) + }) + + // Change to 2 lanes + virtualizer.setOptions({ + count: 10, + lanes: 2, + estimateSize: () => 100, + getScrollElement: () => mockScrollElement, + scrollToFn: vi.fn(), + observeElementRect: (instance, cb) => { + cb({ width: 400, height: 600 }) + return () => {} + }, + observeElementOffset: (instance, cb) => { + cb(0, false) + return () => {} + }, + }) + + virtualizer._willUpdate() + + // Get new measurements with 2 lanes + measurements = virtualizer['getMeasurements']() + expect(measurements.length).toBe(10) + + // All lane assignments should now be 0 or 1 (2 lanes) + // This is the bug fix - previously some items could still have lane: 2 + measurements.forEach((item, index) => { + expect(item.lane).toBeGreaterThanOrEqual(0) + expect(item.lane).toBeLessThan(2) + }) + + // Verify no out of bounds access would occur + const lanes = 2 + const columns = Array.from({ length: lanes }, () => [] as typeof measurements) + measurements.forEach((item) => { + // This should not throw + expect(() => { + columns[item.lane].push(item) + }).not.toThrow() + }) +})