diff --git a/.changeset/thick-mails-carry.md b/.changeset/thick-mails-carry.md new file mode 100644 index 000000000..b66d73fe9 --- /dev/null +++ b/.changeset/thick-mails-carry.md @@ -0,0 +1,5 @@ +--- +'@tanstack/solid-virtual': patch +--- + +Defer non-sync `onChange` updates to a microtask. `measureElement` runs from an item's ref while `` is still iterating the virtual-item store, so the re-entrant `resizeItem` notification reconciled the store array underneath the running `mapArray`, which then read `undefined` items. Scroll-driven updates (`sync: true`) and option changes are still applied synchronously. Reading `getVirtualItems()` or `getTotalSize()` in the same tick as `resizeItem()` or `measure()` now returns the previous value; the update still lands before paint. diff --git a/packages/solid-virtual/src/index.tsx b/packages/solid-virtual/src/index.tsx index 9f16672aa..448ac5f84 100644 --- a/packages/solid-virtual/src/index.tsx +++ b/packages/solid-virtual/src/index.tsx @@ -38,6 +38,36 @@ function createVirtualizerBase< ) const [totalSize, setTotalSize] = createSignal(instance.getTotalSize()) + let pending = false + let disposed = false + onCleanup(() => { + disposed = true + }) + + const flush = () => { + pending = false + if (disposed) return + instance._willUpdate() + setVirtualItems(reconcile(instance.getVirtualItems(), { key: 'index' })) + setTotalSize(instance.getTotalSize()) + } + + const flushIfPending = () => { + if (pending) flush() + } + + // Size changes (`resizeItem`/`measure`, reported with `sync: false`) can + // arrive from a row's ref while is still iterating `virtualItems`, so + // reconciling immediately would mutate the array underneath `mapArray`. + const schedule = (sync: boolean) => { + if (sync) { + flush() + } else if (!pending) { + pending = true + queueMicrotask(flushIfPending) + } + } + const handler = { get( target: Virtualizer, @@ -70,20 +100,12 @@ function createVirtualizerBase< instance: Virtualizer, sync: boolean, ) => { - instance._willUpdate() - setVirtualItems( - reconcile(instance.getVirtualItems(), { - key: 'index', - }), - ) - setTotalSize(instance.getTotalSize()) + schedule(sync) options.onChange?.(instance, sync) }, }), ) - virtualizer._willUpdate() - setVirtualItems(reconcile(instance.getVirtualItems(), { key: 'index' })) - setTotalSize(instance.getTotalSize()) + flush() }) return virtualizer diff --git a/packages/solid-virtual/tests/index.test.ts b/packages/solid-virtual/tests/index.test.ts index 37989150d..7973f3fe1 100644 --- a/packages/solid-virtual/tests/index.test.ts +++ b/packages/solid-virtual/tests/index.test.ts @@ -3,8 +3,8 @@ import { createRoot, createSignal } from 'solid-js' import { createVirtualizer } from '../src/index' -test('preserves measured sizes when reactive options change', () => { - createRoot((dispose) => { +test('preserves measured sizes when reactive options change', async () => { + await createRoot(async (dispose) => { const [count, setCount] = createSignal(2) const virtualizer = createVirtualizer({ get count() { @@ -17,6 +17,8 @@ test('preserves measured sizes when reactive options change', () => { expect(virtualizer.getTotalSize()).toBe(120) virtualizer.resizeItem(0, 100) + // `resizeItem` notifies with `sync: false`, so it lands one microtask later. + await Promise.resolve() expect(virtualizer.getTotalSize()).toBe(160) setCount(3) @@ -26,3 +28,50 @@ test('preserves measured sizes when reactive options change', () => { dispose() }) }) + +test('applies size changes after the current synchronous pass', async () => { + await createRoot(async (dispose) => { + const virtualizer = createVirtualizer({ + count: 100, + getScrollElement: () => null, + estimateSize: () => 50, + initialRect: { width: 100, height: 200 }, + }) + + // The store array iterates: reconcile mutates this exact object. + const items = virtualizer.getVirtualItems() + const before = items.length + expect(before).toBeGreaterThan(1) + expect(virtualizer.getTotalSize()).toBe(100 * 50) + + virtualizer.resizeItem(0, 4000) + expect(items.length).toBe(before) + expect(virtualizer.getTotalSize()).toBe(100 * 50) + + await Promise.resolve() + + expect(items.length).toBeLessThan(before) + expect(virtualizer.getTotalSize()).toBe(4000 + 99 * 50) + dispose() + }) +}) + +test('drops a pending size change after dispose', async () => { + const { virtualizer, items } = createRoot((dispose) => { + const instance = createVirtualizer({ + count: 100, + getScrollElement: () => null, + estimateSize: () => 50, + initialRect: { width: 100, height: 200 }, + }) + const virtualItems = instance.getVirtualItems() + dispose() + return { virtualizer: instance, items: virtualItems } + }) + + const before = items.length + virtualizer.resizeItem(0, 4000) + await Promise.resolve() + + expect(items.length).toBe(before) +}) diff --git a/packages/solid-virtual/tests/measure-during-render.test.tsx b/packages/solid-virtual/tests/measure-during-render.test.tsx new file mode 100644 index 000000000..a64bba8c4 --- /dev/null +++ b/packages/solid-virtual/tests/measure-during-render.test.tsx @@ -0,0 +1,111 @@ +import { expect, test, vi } from 'vitest' +import { For } from 'solid-js' +import { render } from 'solid-js/web' + +import { createVirtualizer } from '../src/index' +import type { Virtualizer } from '../src/index' + +const ROW = 50 +const TALL_ROW = 5000 +const VIEWPORT = 200 +const COUNT = 200 + +// A row measures itself from its ref, which shrinks the rendered range while +// is still iterating the store. The ref writes `data-index` first +// because Solid runs refs before it applies reactive attributes (#930). +function renderList(container: HTMLElement) { + const rowsRendered: Array = [] + let virtualizer!: Virtualizer + let scrollEl!: HTMLDivElement + + function List() { + virtualizer = createVirtualizer({ + count: COUNT, + getScrollElement: () => scrollEl, + estimateSize: () => ROW, + initialRect: { width: VIEWPORT, height: VIEWPORT }, + observeElementRect: (_, cb) => { + cb({ width: VIEWPORT, height: VIEWPORT }) + return () => {} + }, + observeElementOffset: (_, cb) => { + cb(0, false) + return () => {} + }, + // jsdom has no layout; the oversized first row is what shrinks the range. + measureElement: (el) => + el.getAttribute('data-index') === '0' ? TALL_ROW : ROW, + }) + + return ( +
+
+ + {(item) => { + // Unguarded on purpose: a hole in the array throws here. + const index = item.index + rowsRendered.push(index) + return ( +
{ + el.dataset.index = String(index) + virtualizer.measureElement(el) + }} + style={{ + position: 'absolute', + top: 0, + left: 0, + width: '100%', + transform: `translateY(${item.start}px)`, + }} + > + Row {index} +
+ ) + }} +
+
+
+ ) + } + + const dispose = render(() => , container) + return { dispose, rowsRendered, virtualizer: virtualizer! } +} + +const renderedIndexes = (container: HTMLElement) => + Array.from(container.querySelectorAll('[data-index]')).map( + (el) => Number(el.dataset.index), + ) + +test('measuring a row from its ref does not corrupt the pass', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const container = document.createElement('div') + document.body.appendChild(container) + + const { dispose, rowsRendered, virtualizer } = renderList(container) + + const initial = renderedIndexes(container) + expect(initial.length).toBeGreaterThan(2) + expect(rowsRendered).toEqual(initial) + // The measurement was taken (data-index was readable), just not applied yet. + expect(warn).not.toHaveBeenCalled() + expect(virtualizer.itemSizeCache.get(0)).toBe(TALL_ROW) + + await Promise.resolve() + + const settled = renderedIndexes(container) + expect(settled.length).toBeLessThan(initial.length) + expect(settled).toEqual(virtualizer.getVirtualItems().map((i) => i.index)) + expect(virtualizer.getTotalSize()).toBe(TALL_ROW + (COUNT - 1) * ROW) + + dispose() + container.remove() + warn.mockRestore() +}) diff --git a/packages/solid-virtual/vite.config.ts b/packages/solid-virtual/vite.config.ts index 53c000c53..8a7914378 100644 --- a/packages/solid-virtual/vite.config.ts +++ b/packages/solid-virtual/vite.config.ts @@ -1,9 +1,16 @@ import { defineConfig, mergeConfig } from 'vitest/config' import { tanstackViteConfig } from '@tanstack/vite-config' import solid from 'vite-plugin-solid' +import packageJson from './package.json' const config = defineConfig({ plugins: [solid()], + test: { + name: packageJson.name, + dir: './tests', + watch: false, + environment: 'jsdom', + }, }) export default mergeConfig(