fix(core): avoid observer loop errors when resize renders change rows (#1348)

Author: minwookshin

.changeset/quiet-row-observers.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/virtual-core': patch
+---
+
+Avoid ResizeObserver loop errors when a synchronous resize render mounts or unmounts rows. Measure new rows immediately, defer their observation until the next frame, and stop observing detached rows before delivery ends.

packages/react-virtual/e2e/app/resize-delivery/index.html

@@ -0,0 +1,26 @@
+<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <style>
+      :root {
+        --scale: 1;
+      }
+      body {
+        margin: 0;
+        font-family: system-ui, sans-serif;
+      }
+      .row {
+        box-sizing: border-box;
+        padding: 4px 8px;
+        border-top: 1px solid #ddd;
+        font-size: calc(16px * var(--scale));
+        line-height: 1.5;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="root"></div>
+    <script type="module" src="./main.tsx"></script>
+  </body>
+</html>

packages/react-virtual/e2e/app/resize-delivery/main.tsx

@@ -0,0 +1,39 @@
+import React from 'react'
+import ReactDOM from 'react-dom/client'
+import { useVirtualizer } from '@tanstack/react-virtual'
+
+function App() {
+  const parentRef = React.useRef<HTMLDivElement>(null)
+  const virtualizer = useVirtualizer({
+    count: 2000,
+    getScrollElement: () => parentRef.current,
+    estimateSize: () => 33,
+    overscan: 8,
+  })
+
+  return (
+    <div id="scroll" ref={parentRef} style={{ height: 600, overflow: 'auto' }}>
+      <div style={{ height: virtualizer.getTotalSize(), position: 'relative' }}>
+        {virtualizer.getVirtualItems().map((item) => (
+          <div
+            key={item.key}
+            data-index={item.index}
+            ref={virtualizer.measureElement}
+            className="row"
+            style={{
+              position: 'absolute',
+              top: 0,
+              left: 0,
+              width: '100%',
+              transform: `translateY(${item.start}px)`,
+            }}
+          >
+            Row {item.index}
+          </div>
+        ))}
+      </div>
+    </div>
+  )
+}
+
+ReactDOM.createRoot(document.getElementById('root')!).render(<App />)

packages/react-virtual/e2e/app/test/resize-delivery.spec.ts

@@ -0,0 +1,97 @@
+import { expect, test } from '@playwright/test'
+import type { Page } from '@playwright/test'
+
+// Let ResizeObserver delivery and its deferred registrations finish before
+// checking errors. No timer or external resource determines the result.
+const settleFrames = (page: Page) =>
+  page.evaluate(
+    () =>
+      new Promise<void>((resolve) => {
+        let remaining = 5
+        const frame = () => {
+          if (--remaining === 0) resolve()
+          else requestAnimationFrame(frame)
+        }
+        requestAnimationFrame(frame)
+      }),
+  )
+
+for (const direction of ['grow', 'shrink'] as const) {
+  test(`resizing rows can ${direction} the rendered range without observer loop errors`, async ({
+    page,
+  }) => {
+    await page.addInitScript(() => {
+      window.addEventListener('error', (event) => {
+        if (event.message.includes('ResizeObserver loop')) {
+          const root = document.documentElement
+          root.dataset.resizeLoops = String(
+            Number(root.dataset.resizeLoops ?? 0) + 1,
+          )
+        }
+      })
+    })
+    await page.goto('/resize-delivery/')
+    const scroll = page.locator('#scroll')
+    await expect(page.locator('.row').first()).toBeVisible()
+    await scroll.evaluate((element) => {
+      element.scrollTop = element.scrollHeight / 2
+    })
+    await expect
+      .poll(() => page.locator('.row').first().getAttribute('data-index'))
+      .not.toBe('0')
+    await settleFrames(page)
+
+    if (direction === 'grow') {
+      // Growing the rendered range requires shrinking rows. Set up taller
+      // rows first so both paths start at the same scrolled viewport.
+      await page.evaluate(() =>
+        document.documentElement.style.setProperty('--scale', '1.25'),
+      )
+      await settleFrames(page)
+    }
+
+    const errors: Array<string> = []
+    page.on('pageerror', (error) => errors.push(error.message))
+    await page.evaluate(() => {
+      document.documentElement.dataset.resizeLoops = '0'
+    })
+    const countBefore = await page.locator('.row').count()
+    const anchor = await scroll.evaluate((element) => {
+      const top = element.getBoundingClientRect().top
+      const row = [...element.querySelectorAll<HTMLElement>('.row')].find(
+        (item) => {
+          const rect = item.getBoundingClientRect()
+          return rect.top <= top && rect.bottom > top
+        },
+      )!
+      return {
+        index: row.dataset.index!,
+        offset: row.getBoundingClientRect().top - top,
+      }
+    })
+    await page.evaluate(
+      (scale) => document.documentElement.style.setProperty('--scale', scale),
+      direction === 'grow' ? '1' : '1.25',
+    )
+    await expect.poll(() => page.locator('.row').count()).not.toBe(countBefore)
+    await settleFrames(page)
+    const countAfter = await page.locator('.row').count()
+    if (direction === 'grow') expect(countAfter).toBeGreaterThan(countBefore)
+    else expect(countAfter).toBeLessThan(countBefore)
+
+    const anchorBox = await page
+      .locator(`.row[data-index="${anchor.index}"]`)
+      .boundingBox()
+    const scrollBox = await scroll.boundingBox()
+    expect(anchorBox).not.toBeNull()
+    expect(
+      Math.abs(anchorBox!.y - scrollBox!.y - anchor.offset),
+    ).toBeLessThanOrEqual(1)
+    expect(anchorBox!.y).toBeLessThanOrEqual(scrollBox!.y + 1)
+    expect(anchorBox!.y + anchorBox!.height).toBeGreaterThan(scrollBox!.y)
+    expect(await page.locator('html').getAttribute('data-resize-loops')).toBe(
+      '0',
+    )
+    expect(errors).toEqual([])
+  })
+}

packages/react-virtual/e2e/app/vite.config.ts

@@ -16,6 +16,10 @@ export default defineConfig({
           __dirname,
           'measure-element/index.html',
         ),
+        'resize-delivery': path.resolve(
+          __dirname,
+          'resize-delivery/index.html',
+        ),
         'smooth-scroll': path.resolve(__dirname, 'smooth-scroll/index.html'),
         'smooth-prepend': path.resolve(__dirname, 'smooth-prepend/index.html'),
         'stale-index': path.resolve(__dirname, 'stale-index/index.html'),

packages/virtual-core/src/index.ts

@@ -642,6 +642,9 @@ export class Virtualizer<
   private observer = (() => {
     let _ro: ResizeObserver | null = null
     let _frames: ReturnType<typeof createFrameQueue> | null = null
+    let delivering = false
+    let observeQueued = false
+    const pendingObservations = new Set<Element>()
 
     const get = () => {
       if (_ro) {
@@ -672,12 +675,23 @@ export class Virtualizer<
               this._noteClampMax()
               this._flushHoldIfReady(true)
               this._retryClampedAdjustment()
+              // Ref-null callbacks run before the host nodes are detached.
+              // Drop them after the sync renders, before the browser gathers
+              // another round of ResizeObserver entries at their new depth.
+              this.measureElement(null)
             }
           }
         }
-        this.options.useAnimationFrameWithResizeObserver
-          ? frames.schedule(run)
-          : run()
+        if (this.options.useAnimationFrameWithResizeObserver) {
+          frames.schedule(run)
+        } else {
+          delivering = true
+          try {
+            run()
+          } finally {
+            delivering = false
+          }
+        }
       }))
     }
 
@@ -715,12 +729,37 @@ export class Virtualizer<
       disconnect: () => {
         get()?.disconnect()
         _frames?.cancel()
+        pendingObservations.clear()
+        observeQueued = false
         _ro = null
         _frames = null
       },
-      observe: (target: Element) =>
-        get()?.observe(target, { box: 'border-box' }),
-      unobserve: (target: Element) => get()?.unobserve(target),
+      observe: (target: Element) => {
+        const observer = get()
+        if (!observer) return
+        if (!delivering) {
+          observer.observe(target, { box: 'border-box' })
+          return
+        }
+        // A sync render can also mount rows at the depth being delivered.
+        // A microtask is still inside that delivery: register next frame,
+        // while measureElement continues to measure the new row immediately.
+        pendingObservations.add(target)
+        if (!observeQueued) {
+          observeQueued = true
+          _frames?.schedule(() => {
+            observeQueued = false
+            pendingObservations.forEach((node) => {
+              if (node.isConnected) _ro?.observe(node, { box: 'border-box' })
+            })
+            pendingObservations.clear()
+          })
+        }
+      },
+      unobserve: (target: Element) => {
+        pendingObservations.delete(target)
+        get()?.unobserve(target)
+      },
     }
   })()
   range: { startIndex: number; endIndex: number } | null = null

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

@@ -736,13 +736,20 @@ test('changing gap invalidates cached measurements (multi-lane)', () => {
 
 function setupResizeBatch(onChange: (instance: any, sync: boolean) => void) {
   let roCallback: ResizeObserverCallback | null = null
+  const observe = vi.fn()
+  const unobserve = vi.fn()
+  const frames = new Map<number, FrameRequestCallback>()
+  let frameId = 0
   const mockWindow = {
-    requestAnimationFrame: vi.fn(),
-    cancelAnimationFrame: vi.fn(),
+    requestAnimationFrame: vi.fn((callback: FrameRequestCallback) => {
+      frames.set(++frameId, callback)
+      return frameId
+    }),
+    cancelAnimationFrame: vi.fn((id: number) => frames.delete(id)),
     performance: { now: () => Date.now() },
     ResizeObserver: vi.fn(function (cb: ResizeObserverCallback) {
       roCallback = cb
-      return { observe: vi.fn(), unobserve: vi.fn(), disconnect: vi.fn() }
+      return { observe, unobserve, disconnect: vi.fn() }
     }),
   }
   const el = {
@@ -811,7 +818,21 @@ function setupResizeBatch(onChange: (instance: any, sync: boolean) => void) {
       ),
       {} as ResizeObserver,
     )
-  return { virtualizer, sizes, node, deliver }
+  const flush = () => {
+    const queued = [...frames.values()]
+    frames.clear()
+    queued.forEach((callback) => callback(0))
+  }
+  return {
+    virtualizer,
+    sizes,
+    node,
+    deliver,
+    observe,
+    unobserve,
+    frames,
+    flush,
+  }
 }
 
 test('ResizeObserver batch: later entries see starts updated by earlier ones (#1218)', () => {
@@ -850,6 +871,70 @@ test('ResizeObserver batch: a sync re-render does not drop queued entries (#1036
   expect(notifies).toEqual([true])
 })
 
+test('unobserves rows detached by a synchronous resize render before delivery ends', () => {
+  const { virtualizer, sizes, node, deliver, unobserve } = setupResizeBatch(
+    (instance, sync) => {
+      if (!sync) return
+      // React calls the ref with null before removing the host element.
+      instance.measureElement(null)
+      node(5).isConnected = false
+    },
+  )
+  sizes.set(3, 200)
+  deliver([3])
+  expect(unobserve).toHaveBeenCalledWith(node(5))
+  expect(virtualizer.elementsCache.has(5)).toBe(false)
+})
+
+test('measures new rows synchronously but observes them after resize delivery', () => {
+  const { virtualizer, sizes, node, deliver, observe, frames, flush } =
+    setupResizeBatch((instance, sync) => {
+      if (sync) {
+        instance.measureElement(node(10))
+        instance.measureElement(node(11))
+      }
+    })
+  sizes.set(10, 120)
+  sizes.set(11, 130)
+  sizes.set(3, 200)
+  deliver([3])
+  expect(virtualizer.getMeasurements()[10]!.size).toBe(120)
+  expect(virtualizer.getMeasurements()[11]!.size).toBe(130)
+  expect(observe.mock.calls.map(([target]) => target)).not.toContain(node(10))
+  expect(observe.mock.calls.map(([target]) => target)).not.toContain(node(11))
+  expect(frames.size).toBe(1)
+  flush()
+  expect(observe).toHaveBeenCalledWith(node(10), { box: 'border-box' })
+  expect(observe).toHaveBeenCalledWith(node(11), { box: 'border-box' })
+})
+
+test.each(['removed', 'replaced', 'disconnected'] as const)(
+  'does not register a deferred row after it is %s',
+  (action) => {
+    const { virtualizer, sizes, node, deliver, observe, flush } =
+      setupResizeBatch((instance, sync) => {
+        if (sync) instance.measureElement(node(10))
+      })
+    sizes.set(10, 120)
+    sizes.set(3, 200)
+    deliver([3])
+    expect(observe.mock.calls.map(([target]) => target)).not.toContain(node(10))
+
+    if (action === 'removed') {
+      node(10).isConnected = false
+      virtualizer.measureElement(null)
+    } else if (action === 'replaced') {
+      const replacement = { ...node(10) }
+      virtualizer.measureElement(replacement)
+      expect(observe).toHaveBeenCalledWith(replacement, { box: 'border-box' })
+    } else {
+      virtualizer._didMount()()
+    }
+    flush()
+    expect(observe.mock.calls.map(([target]) => target)).not.toContain(node(10))
+  },
+)
+
 // ─── useAnimationFrameWithResizeObserver: frames of the scroll element's window
 
 // A window other than the global one, as for a list rendered into a