fix(virtual-core): record the committed max as a clamp candidate (#1330)

Author: piecyk

.changeset/clamped-commit-survives.md

@@ -0,0 +1,7 @@
+---
+'@tanstack/virtual-core': patch
+---
+
+fix(virtual-core): keep an instant scroll command clamped at a max the sizer committed outside a row resize
+
+Before an instant scroll command's write is reported, the browser can clamp it at a smaller scroll max that the sizer moves to without a row resize: an append or prepend, `measure()`, or a resize the sizer had not committed when the command was written. The virtualizer now also records the max at each commit, so such an event is read as its own clamped write and the command lands. A clamped write reported with `isScrolling` false is now written again too.

packages/virtual-core/src/index.ts

@@ -610,15 +610,16 @@ export class Virtualizer<
   //   further by the time it reports.
   // - `smooth`: the write animates, so until it lands the element is
   //   somewhere between the old offset and `target`.
-  // - `max`, `lowMaxes`: for a command's instant write, the scroll max as
-  //   rows measured since the write moved it (`resizeItem`), and each value
-  //   it took below `target`. The browser clamps the write to one of them
-  //   at a layout, and the sizer can grow back before the report, so the
-  //   read-back is that max rather than the element's current one.
+  // - `lowMaxes`: for a command's instant write, each scroll max below
+  //   `target` the element had since the write. A command's target is
+  //   clamped to the max at the write, but the sizer can shrink under it
+  //   before the report (rows measured smaller, a count change) and grow
+  //   back, so the read-back is the max the browser clamped to then rather
+  //   than the element's current one. Read wherever the sizer can have
+  //   been committed (`_noteClampMax`).
   private pendingWrite: {
     target: number
     smooth: boolean
-    max?: number
     lowMaxes?: Array<number>
   } | null = null
   // A compensation write from `applyScrollAdjustment` whose target exceeded
@@ -673,6 +674,7 @@ export class Virtualizer<
             this._resizeBatch = null
             if (batch.notify) {
               this.notify(batch.sync)
+              this._noteClampMax()
               this._flushHoldIfReady(true)
               this._retryClampedAdjustment()
             }
@@ -1094,8 +1096,8 @@ export class Virtualizer<
           } else if (
             kind === 'clampedReadBack' &&
             this.scrollState?.index != null &&
-            elementMax !== null &&
-            offset < elementMax + this._holdOffset - 1.5
+            // Not `elementMax`: an adapter can report the read-back at rest.
+            offset < this.getMaxScrollOffset() - 1.5
           ) {
             // Our write, clamped at a max the sizer has since grown back
             // from: reconcile writes the target again, even when the layout
@@ -1212,6 +1214,9 @@ export class Virtualizer<
       this._scrollToOffset(this.getScrollOffset())
     }
 
+    // The consumer has committed the sizer.
+    this._noteClampMax()
+
     const anchor = this.pendingScrollAnchor
     this.pendingScrollAnchor = null
 
@@ -2048,30 +2053,6 @@ export class Virtualizer<
       this.itemSizeCache.set(key, size)
       this.itemSizeCacheVersion++
 
-      // Follow the scroll max through the sizes measured before a command's
-      // instant write reports back (see `pendingWrite`). The element's max
-      // is read once, at the first resize after the write, before that
-      // resize reaches the sizer; it may already be below the target. With
-      // lanes, a row's delta is not the total's, so nothing is followed.
-      const write =
-        this.options.lanes === 1 && this._awaitingCommandReadBack()
-          ? this.pendingWrite!
-          : null
-      const followMax = (max: number) => {
-        write!.max = max
-        // The element cannot clamp below its own 0, which is the hold in
-        // layout coordinates; the raw max stays for the next delta.
-        const reachable = Math.max(max, this._holdOffset)
-        if (reachable < write!.target) {
-          ;(write!.lowMaxes ??= []).push(reachable)
-        }
-      }
-      if (write !== null && write.max === undefined) {
-        const elementMax = this._elementMaxScrollOffset()
-        if (elementMax !== null) followMax(elementMax + this._holdOffset)
-      }
-      if (write?.max !== undefined) followMax(write.max + delta)
-
       // Before a command's read-back, an index or end target is retargeted
       // from fresh measurements by reconcileScroll next frame; an offset
       // target lands where it was written and the delta is not applied.
@@ -2104,6 +2085,7 @@ export class Virtualizer<
         return
       }
       this.notify(adjustedSync)
+      this._noteClampMax()
       // A consumer that grows the sizer synchronously inside `onChange`
       // (direct DOM updates) may never re-render when the range is
       // unchanged, so retry a clamped write here rather than only in
@@ -2164,6 +2146,23 @@ export class Virtualizer<
     return notUndefined(measurements[idx])
   }
 
+  // Record the element's max, in layout coordinates, as one a command's
+  // pending instant write may be clamped to (see `lowMaxes` on
+  // `pendingWrite`). Called where the sizer can have been committed: after
+  // a notify, for a consumer that sizes it in `onChange`, and in
+  // `_willUpdate`. The element cannot clamp below its own 0, which is the
+  // hold in layout coordinates.
+  private _noteClampMax = () => {
+    if (!this._awaitingCommandReadBack()) return
+    const elementMax = this._elementMaxScrollOffset()
+    if (elementMax === null) return
+    const write = this.pendingWrite!
+    const max = Math.max(elementMax, 0) + this._holdOffset
+    if (max >= write.target) return
+    const lowMaxes = (write.lowMaxes ??= [])
+    if (lowMaxes[lowMaxes.length - 1] !== max) lowMaxes.push(max)
+  }
+
   // The element's scroll range in layout coordinates: while a correction is
   // held the sizer is short by the hold, so the element's own max is too.
   private getMaxScrollOffset = () =>

packages/virtual-core/src/scroll-classifier.ts

@@ -34,7 +34,7 @@ export interface ScrollEventInput {
   // The layout position the virtualizer holds.
   scrollOffset: number | null
   // Our last write to the element, until a scroll event reports it, with
-  // the scroll maxima below its target the layout took since (see
+  // the scroll maxima below its target the element had since (see
   // `lowMaxes` on `pendingWrite` in the virtualizer).
   pendingWrite: {
     target: number
@@ -81,11 +81,11 @@ export function classifyScrollEvent({
   ) {
     return 'clampedReadBack'
   }
-  // The same, clamped to a smaller max than the element's current one: a row
-  // measured smaller shrank the sizer below the target and the browser
-  // clamped there, then a row measured larger grew it back, all before the
-  // report. The clamp lands on one of those maxima; a position between them
-  // is not one.
+  // The same, clamped to a smaller max than the element's current one: the
+  // sizer shrank below the target after the write (rows measured smaller,
+  // a count change) and the browser clamped there, then it grew back, all
+  // before the report. The clamp lands on one of those maxima; a position
+  // between them is not one.
   if (
     pendingWrite !== null &&
     !pendingWrite.smooth &&

packages/virtual-core/tests/__snapshots__/scroll-trace.test.ts.snap

@@ -66,6 +66,39 @@ exports[`scroll behaviour snapshot > base web 1`] = `
 ]
 `;
 
+exports[`scroll behaviour snapshot > commitPerRow ios 1`] = `
+[
+  "0: 188b7eb6 cbb399d1 0c68f8d8 52d44e85 030fdf66 f89bae5e e33eb3b7 4b11cbb6 bc3f4dfa 0d8fe7ec",
+  "10: 06bda0c7 fbdcf4b6 92b9b7d7 ac880e66 9880cb46 5ff7a292 10a7d3a0 78eed177 e0b1c0ac e25c37ec",
+  "20: 7d64b6f6 8ddb843c 01a561c0 4f8fccf5 06aa4f85 0c141569 cfbd97d3 aa353506 4634aede 5052c204",
+  "30: b6a919af 84dc4560 7e492fad 1852617f b06693e3 124a7157 f88d78a2 a89b2b4f 2d610d8a 155761e2",
+  "40: 0d274a15 5b099780 9dcf3736 54322109 bf57934a 6041f019 b28f19d9 e5e20e3b cfb551ab 2c1bbec2",
+  "50: ad0cbc1d 1f82fb9b 8ab02f8b 23c2e2fb 8cb40f92 ec29f0fd 91528992 14e26564 884a1fff 622e658e",
+]
+`;
+
+exports[`scroll behaviour snapshot > commitPerRow ios27 1`] = `
+[
+  "0: e9f2ce2c cbb399d1 0c68f8d8 2c24151f 030fdf66 43163cec e33eb3b7 0c7f462d bc3f4dfa 0d8fe7ec",
+  "10: 9ee9150b fbdcf4b6 92b9b7d7 ec0a38f6 9880cb46 60d448ce 10a7d3a0 78eed177 e0b1c0ac a5e63aaf",
+  "20: 7d64b6f6 8ddb843c 01a561c0 6c999c46 3dcaee6a b2e63a21 cfbd97d3 b6a51eb6 e50f28ad 5052c204",
+  "30: b6a919af 84dc4560 42933fed 1852617f b06693e3 124a7157 13948bf0 a89b2b4f 2d610d8a 155761e2",
+  "40: 019380ed 5b099780 9dcf3736 c80d856e bf57934a 6041f019 c20f0271 e5e20e3b cfb551ab 2c1bbec2",
+  "50: ad0cbc1d 1f82fb9b 8ab02f8b 23c2e2fb 8cb40f92 d54527a4 7989082d 14e26564 884a1fff 98ba9a64",
+]
+`;
+
+exports[`scroll behaviour snapshot > commitPerRow web 1`] = `
+[
+  "0: ceb7f0e7 cbb399d1 f4a63add a132747c 030fdf66 643d2755 e33eb3b7 4b11cbb6 bc3f4dfa 0d8fe7ec",
+  "10: 06bda0c7 fbdcf4b6 166319fd ac880e66 9880cb46 5ff7a292 31061954 dae8df99 e2cc19b7 e25c37ec",
+  "20: 7e86e2b6 fe1ff262 01a561c0 f91f4bae 6e38d09b 4ee9d5dd 7135be90 4ae76f75 b13f2c93 5052c204",
+  "30: b6a919af 2109d992 7e492fad 1852617f b06693e3 124a7157 117f3ed1 a89b2b4f 2d610d8a 155761e2",
+  "40: 0d274a15 5b099780 2c1ceef6 f813aca3 bf57934a 8e53c5c4 de439e3e e5e20e3b 5e08526b ef4b633c",
+  "50: 26d62359 e308f594 a0ad4db6 9d7ab115 509fe1e1 53afc076 83a11448 14e26564 884a1fff 622e658e",
+]
+`;
+
 exports[`scroll behaviour snapshot > horizontal ios 1`] = `
 [
   "0: a9fcb9d4 0d2cec68 ed35961e 0a69fa03 0be21da6 3ec8eb85 869fa2d1 27716a81 f885c716 44f33e92",