fix(useRecycleScroller): prevent blank rows during transient cache states, fix #906

Author: Akryum

packages/vue-virtual-scroller/src/composables/useRecycleScroller.spec.ts

@@ -1154,6 +1154,70 @@ describe('useRecycleScroller', () => {
     expect(recoveryRange[1]).toBeGreaterThanOrEqual(baselineRange[1])
   })
 
+  it('still assigns a view for an in-range index whose cached size is 0', async () => {
+    // Regression for issue #906: the step-2 loop in updateVisibleItems used
+    // to early-return when `sizesValue[i].size` was 0 (or `sizesValue[i]`
+    // was undefined), skipping view assignment for that index. Combined
+    // with step 1 having already recycled every prior view (on the
+    // itemsChanged / non-continuous paths), the DOM slot was left blank
+    // until the next reconciliation tick. The fix falls back to
+    // `_computedMinItemSize` so every index in the resolved range claims
+    // a pooled view regardless of cache transients.
+    const { vm } = mountHarness({
+      items: Array.from({ length: 5 }, (_, id) => ({ id, size: 20 })),
+      itemSize: null,
+      minItemSize: 20,
+      clientHeight: 100,
+    })
+
+    await nextTick()
+    await nextTick()
+
+    // Sanity: all five items should be in the visible pool to start with.
+    const initialIndices = vm.visiblePool.map((view: View) => view.nr.index)
+    expect(initialIndices).toEqual([0, 1, 2, 3, 4])
+
+    // Punch size=0 into the middle of the cache. The accumulators stay
+    // populated so the binary search still includes index 2 in the range —
+    // we want to exercise the per-index assignment loop, not the gate.
+    const sizesRef = vm.sizes as Array<{ accumulator: number, size: number | undefined } | undefined>
+    const savedSize = sizesRef[2]!.size
+    sizesRef[2]!.size = 0
+
+    vm.updateVisibleItems(true)
+
+    sizesRef[2]!.size = savedSize
+
+    const recoveryIndices = vm.visiblePool.map((view: View) => view.nr.index)
+    expect(recoveryIndices).toContain(2)
+  })
+
+  it('still assigns a view when an in-range sizesValue entry is undefined', async () => {
+    // Same regression as the size=0 case (issue #906), but exercising the
+    // `sizesValue[i] && ...` branch. The fallback must catch undefined
+    // entries too — a sparse cache slot would otherwise also skip the slot.
+    const { vm } = mountHarness({
+      items: Array.from({ length: 5 }, (_, id) => ({ id, size: 20 })),
+      itemSize: null,
+      minItemSize: 20,
+      clientHeight: 100,
+    })
+
+    await nextTick()
+    await nextTick()
+
+    const sizesRef = vm.sizes as Array<{ accumulator: number, size: number | undefined } | undefined>
+    const saved = sizesRef[2]
+    sizesRef[2] = undefined
+
+    vm.updateVisibleItems(true)
+
+    sizesRef[2] = saved
+
+    const recoveryIndices = vm.visiblePool.map((view: View) => view.nr.index)
+    expect(recoveryIndices).toContain(2)
+  })
+
   it('does not crash on 0 → 1 items transition in variable-size mode', async () => {
     // Regression: in variable-size mode the size cache is computed lazily from
     // `items`. When an empty list gains its first row, an upstream wrapper can

packages/vue-virtual-scroller/src/composables/useRecycleScroller.ts

@@ -1468,8 +1468,13 @@ export function useRecycleScroller<TOptions extends UseRecycleScrollerOptions<an
           const viewVisible = renderedIndexSet
             ? renderedIndexSet.has(view.nr.index)
             : (view.nr.index >= startIndex && view.nr.index < endIndex)
-          const viewSize = itemSize || (sizesValue[view.nr.index] && sizesValue[view.nr.index].size)
-          if (!viewVisible || !viewSize) {
+          // Recycle only when the view's index has left the visible range.
+          // A 0 / undefined cached size for a still-visible row is a
+          // transient state (e.g. an item that hasn't been measured yet);
+          // recycling on it would force step 2 to immediately re-claim the
+          // slot, and any per-tick gap visible to the browser shows up as
+          // a blank row. See issue #906.
+          if (!viewVisible) {
             removeAndRecycleView(view, keepFlowModeOrderIncrementally)
           }
         }
@@ -1493,9 +1498,15 @@ export function useRecycleScroller<TOptions extends UseRecycleScrollerOptions<an
     let firstVisiblePosition: number | null = null
     let renderedVisibleSize = 0
     forEachRenderedIndex(startIndex, endIndex, renderedIndices, (i) => {
-      const elementSize = itemSize || (sizesValue[i] && sizesValue[i].size)
-      if (!elementSize)
-        return
+      // Fall back to `_computedMinItemSize` (and ultimately `1`) so every
+      // index in the resolved range claims a pooled view even when the
+      // cache is transiently sparse or reports `size: 0` for an unmeasured
+      // row. Returning early here would leave the DOM slot blank until
+      // the next reconciliation tick — exactly the "blank rows" symptom
+      // reported in issue #906, made worse by `scrollToItem` jumps which
+      // recycle every view in step 1 and rely on this loop to repopulate.
+      const cachedSize = sizesValue[i] && sizesValue[i].size
+      const elementSize = itemSize || cachedSize || _computedMinItemSize || 1
       item = currentItems[i]
       const key = (keyField ? resolveItemKey(item, i, keyField) : i) as ItemKey<TItem, TKeyField>
       view = views.get(key)

tests/e2e/dynamic-scroller.spec.ts

@@ -6,6 +6,7 @@ import {
   metric,
   readMetricNumbers,
   scrollViewportBy,
+  viewport,
   waitForSettle,
 } from './support/demo'
 
@@ -86,6 +87,34 @@ test('dynamic scroller demo keeps visible rows contiguous after fast scrolling',
   }
 })
 
+test('dynamic scroller demo keeps visible rows contiguous after a far absolute jump', async ({ browserName, page }) => {
+  test.skip(browserName !== 'chromium')
+
+  // Regression for issue #906: a single large jump (the same code path
+  // `scrollToItem` uses internally) recycles every visible view in
+  // updateVisibleItems step 1 and must repopulate the slots in step 2,
+  // even for rows whose size cache hasn't measured yet. A skipped index
+  // would surface here as a non-contiguous row gap.
+  await page.goto('/demos/dynamic-scroller')
+  await waitForSettle(page)
+
+  await viewport(page).evaluate((element) => {
+    const target = element as HTMLElement
+    target.scrollTop = Math.max(0, Math.floor((target.scrollHeight - target.clientHeight) * 0.5))
+    target.dispatchEvent(new Event('scroll'))
+  })
+  await waitForSettle(page)
+  await waitForContiguousVisibleRows(page)
+
+  await viewport(page).evaluate((element) => {
+    const target = element as HTMLElement
+    target.scrollTop = Math.max(0, target.scrollHeight - target.clientHeight - 100)
+    target.dispatchEvent(new Event('scroll'))
+  })
+  await waitForSettle(page)
+  await waitForContiguousVisibleRows(page)
+})
+
 test('dynamic scroller demo filters, remeasures, and updates the visible range', async ({ browserName, page }) => {
   test.skip(browserName !== 'chromium')