fix(solid-virtual): defer non-sync onChange updates to a microtask

.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 `<For>` 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.

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 <For> 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<TScrollElement, TItemElement>,
@@ -70,20 +100,12 @@ function createVirtualizerBase<
           instance: Virtualizer<TScrollElement, TItemElement>,
           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

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<HTMLDivElement, HTMLDivElement>({
       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<HTMLDivElement, HTMLDivElement>({
+      count: 100,
+      getScrollElement: () => null,
+      estimateSize: () => 50,
+      initialRect: { width: 100, height: 200 },
+    })
+
+    // The store array <For> 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<HTMLDivElement, HTMLDivElement>({
+      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)
+})

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
+// <For> 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<number> = []
+  let virtualizer!: Virtualizer<HTMLDivElement, HTMLDivElement>
+  let scrollEl!: HTMLDivElement
+
+  function List() {
+    virtualizer = createVirtualizer<HTMLDivElement, HTMLDivElement>({
+      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 (
+      <div ref={scrollEl} style={{ height: `${VIEWPORT}px`, overflow: 'auto' }}>
+        <div
+          style={{
+            height: `${virtualizer.getTotalSize()}px`,
+            position: 'relative',
+          }}
+        >
+          <For each={virtualizer.getVirtualItems()}>
+            {(item) => {
+              // Unguarded on purpose: a hole in the array throws here.
+              const index = item.index
+              rowsRendered.push(index)
+              return (
+                <div
+                  data-index={index}
+                  ref={(el) => {
+                    el.dataset.index = String(index)
+                    virtualizer.measureElement(el)
+                  }}
+                  style={{
+                    position: 'absolute',
+                    top: 0,
+                    left: 0,
+                    width: '100%',
+                    transform: `translateY(${item.start}px)`,
+                  }}
+                >
+                  Row {index}
+                </div>
+              )
+            }}
+          </For>
+        </div>
+      </div>
+    )
+  }
+
+  const dispose = render(() => <List />, container)
+  return { dispose, rowsRendered, virtualizer: virtualizer! }
+}
+
+const renderedIndexes = (container: HTMLElement) =>
+  Array.from(container.querySelectorAll<HTMLElement>('[data-index]')).map(
+    (el) => Number(el.dataset.index),
+  )
+
+test('measuring a row from its ref does not corrupt the <For> 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()
+})

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(