Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Select an option

  • Save tclem/a0449c54061d4a8942e6f61f232a4880 to your computer and use it in GitHub Desktop.

Select an option

Save tclem/a0449c54061d4a8942e6f61f232a4880 to your computer and use it in GitHub Desktop.
Could the v3 diff surface use TanStack Virtual? — a structural analysis (re: github/github-app#4382)

Could v3 use TanStack Virtual? — a structural analysis

Question. PR #4382 — Rewrite the diff surface (v3): O(1) hot paths regardless of diff size replaced an imperative renderer (RowRenderer) and a hand-rolled transform-scroll layer (useTransformScroll) for the v2 surface that was built on @tanstack/react-virtual. Is it really not possible to make TanStack Virtual (tanstack.com/virtual) work for this surface?

Short answer. It is possible in the trivial sense that you can compile a TanStack Virtual–backed diff surface that scrolls. It is not possible to do so without giving up at least three of the eight architectural laws that v3 was deliberately built to satisfy. v2 itself is the existence proof: it was a TanStack Virtual surface, and it is exactly the surface v3 was written to replace.

This report walks through the conflict point by point, with citations, and then maps out the (small) parts of the diff surface where TanStack Virtual would still be a fine choice.


1. v2 was the TanStack Virtual implementation. v3 walked away on purpose.

The v2 AllFilesDiffSurface is the existing canonical TanStack-driven diff surface in this codebase. It still imports useVirtualizer and constructs one over the entire displayRows array:

src/components/diff-surface/AllFilesDiffSurface.tsx:4-5, 985-993

import { useVirtualizer } from "@tanstack/react-virtual";

const virtualizer = useVirtualizer<HTMLDivElement, HTMLDivElement>({
    count: displayRows.length,
    getScrollElement: () => scrollContainerRef.current,
    estimateSize: rowHeight,
    overscan: OVERSCAN_ROWS,
    getItemKey,
    useFlushSync: false,
    rangeExtractor,
});

The PR description (#4382) explicitly frames v3 as a response to v2's behavior in this configuration:

"AllFilesDiffSurface (v2) was 4,300 lines and ~110 hooks. Open time, scroll cost, and click-to-scroll latency all scaled with totalRows and totalFiles. … Profiling showed the root cause was architectural, not a hot loop: heights were measured from the DOM and fed back into layout, which guaranteed measurement loops, cascade rebuilds on chunk arrival, and React renders on every scroll frame."

PR #4382 description

The architecture doc is more direct:

"On large diffs (10k+ rows) FPS collapsed to single digits during fling scroll, layout-thrash from per-row ResizeObserver feedback piled up, and 100+ MB of resident memory came from non-recycled row trees. v2 also had a 'wrong file lands at the top' bug whenever slot heights shifted after scrollToFile queued a target offset."

docs/diff-surface-v3-architecture.md, §1

The earlier docs/diff-surface-v3-plan.md shows the decision was a conscious trade-off, not a knee-jerk:

"Risk: TanStack Virtual misbehaves under our estimateSize strategy at 500K items. Mitigation: Pre-validated in Stage 2 with the real-history Tier 2 specs before threading parity work begins. If TanStack Virtual breaks at this scale, we own the virtualizer too — write a minimal one. Documented as an architecture-decision-required moment."

docs/diff-surface-v3-plan.md, risk register

That escape hatch is what v3 became: useTransformScroll + RowRenderer + displayOffsets are the minimal virtualizer the plan permitted.


2. The conflict, mapped to the v3 laws

The v3 contract is codified in skills/diff-surface-v3/SKILL.md as eight hard laws. Each of the following violations is independent — even if you fix one, the next one still kills the design.

Conflict A — TanStack Virtual publishes scroll state to React (violates Law 5: "React renders structure; the renderer paints rows")

useVirtualizer is, by construction, a React hook that maintains a range state and exposes it via getVirtualItems(). When the underlying scroll element fires a scroll event, the virtualizer updates its range and triggers a re-render of the component that called the hook. The standard rendering idiom in TanStack Virtual is then:

{virtualizer.getVirtualItems().map(item => <Row key={item.key}  />)}

That .map produces O(window) React reconciliation work per scroll frame. v3's Law 5 is exactly the inverse: the React tree must not re-render on scroll. The probe surfaceRenderCount in probes.ts exists specifically to enforce this, and the contract states it must stay bounded "across many scroll steps":

src/components/diff-surface/v3/probes.ts:39-40

/** Number of times `DiffSurfaceV3`'s render body executed. */
surfaceRenderCount: number;

The v3 path achieves this by routing scroll through useTransformScroll, which writes style.transform = translate3d(0, -y, 0) directly on the content layer from the proxy's onScroll handler — no React state update for the transform, only an onScrollChange callback the host wires straight into the imperative RowRenderer.update(...). See:

src/components/diff-surface/v3/render/useTransformScroll.ts:109-132

const writeTransform = useCallback(
    (logical: number) => {
        const layer = contentLayerRef.current;
        if (!layer) return;
        layer.style.transform = `translate3d(0, ${-logical}px, 0)`;
    },
    [contentLayerRef],
);

const onProxyScroll = useCallback(() => {
    const proxy = proxyRef.current;
    if (!proxy) return;
    const proxyTop = proxy.scrollTop;
    const raw = ratio === 1 ? proxyTop : Math.round(proxyTop * ratio);
    const logical = Math.min(raw, logicalMax);
    writeTransform(logical);
    if (logical !== logicalRef.current) {
        logicalRef.current = logical;
        setLogicalScrollTop(logical);
    }
}, [proxyRef, ratio, logicalMax, writeTransform]);

logicalScrollTop is React state, but consumers that want zero-render scroll wire onScrollChange (callback, no state) instead. With TanStack Virtual you can't opt out — the hook's whole API contract is "I tell you when the visible window changed by re-rendering you."

A clever consumer can push the virtualizer into a deeply-memoized leaf so the upstream tree doesn't rerender, but the leaf containing useVirtualizer will re-render at scroll cadence, and inside it the .map(virtualItems) will iterate. v3 wants zero per-scroll React work, not relocated React work.

Conflict B — Per-row React rendering vs imperative innerHTML (violates the soft rule against per-row React + Law 6 mounted-DOM bound)

TanStack Virtual's intended usage is per-row React components. v3 specifically beats reconciliation by building rows as raw HTML strings via buildRowHtml and inserting them with one parseRowHtml + appendChild per visible-window delta:

src/components/diff-surface/v3/render/RowRenderer.ts:325-332

// One batched parse + appendChild.
const fragment = parseRowHtml(wantedHtml.join(""));
const newNodes: HTMLDivElement[] = [];
for (const node of Array.from(fragment.children)) {
    newNodes.push(node as HTMLDivElement);
}
this.container.appendChild(fragment);

And the recycle gate that keeps rowsMountedNow < ~100 regardless of diff size:

src/components/diff-surface/v3/render/RowRenderer.ts:269-287 (recycle hit if sameKind && sameRow && existing.heightPx === height && tokenStateMatches && searchStateMatches && formatStateMatches)

Each diff row is not just text — it's a syntax-highlighted, search-highlight-decorated, split-view-format-aware tokenized string. Routing that through React per row means either:

  1. dangerouslySetInnerHTML per row (you've recreated the imperative path inside React, with extra reconciliation overhead and no recycling), or
  2. Per-token <span>.map(...) inside each row (this is exactly the per-row React component anti-pattern called out in SKILL.md's anti-pattern table — "Per-row React component rendering tokens" → "reconciliation dominates scroll" → "imperative buildRowHtml string").

There is no third option that avoids both, so long as TanStack's hook is the source of the visible window.

Conflict C — Dynamic measurement is TanStack's escape hatch; v3 forbids it entirely (violates Laws 1 and 2)

Any time someone reaches for TanStack Virtual on rows of unknown height, the path is measureElement (under the hood: ResizeObserver-based remeasurement), with TanStack reconciling the cumulative offset table after the fact. From the official TanStack Virtual docs:

"If you're rendering content with dynamic sizes, the virtualizer needs to measure each item to know its size. The recommended way is to use the measureElement API …"

tanstack.com/virtual/latest, Variable Sizes

v3 explicitly bans this:

[SKILL.md Law 1] "Heights are computed deterministically from manifest data + slot data + collapse state + wrap state. The single source of truth is estimateDisplayItemHeight() in v3HeightConstants.ts. No DOM measurement, no getBoundingClientRect, no offsetHeight reads anywhere in the layout pipeline."

[SKILL.md Law 2] "ResizeObserver on slots, rows, headers, or the content layer is forbidden as a height-feedback mechanism. v3 does not run them — heights are deterministic, so scrollToIndex lands exactly on first call."

The motivation isn't aesthetic — it's the v2 "wrong file lands at the top" bug, where scrollToFile would resolve a target offset, the virtualizer would mount the rows, ResizeObserver would re-measure them, the offset table would shift, and the user would land on the wrong file. v2 layered a workaround on top of TanStack via shouldAdjustScrollPositionOnItemSizeChange to suppress some of this:

src/components/diff-surface/AllFilesDiffSurface.tsx:1013-1024

useEffect(() => {
    virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) =>
        shouldAdjustScrollPositionForMeasuredRow({
            itemStart: item.start,
            scrollOffset: instance.scrollOffset ?? 0,
            isDynamicRow: isDynamicDisplayRow(displayRows[item.index]),
        });
}, [displayRows, isDynamicDisplayRow, virtualizer]);

That is the design tension surfacing as a hook callback: "please don't use the measurement you took to correct my scroll position." v3 fixes it at the source — there is no measurement to ignore, because heights are computed from data.

You can run TanStack Virtual in pure-estimateSize mode and never call measureElement, but at that point the virtualizer's value-add reduces to "give me the visible window from a prefix sum I already gave you." That is roughly 80 lines of code we already have (displayOffsets.ts + displayIndexAtPx.ts), and you would still inherit Conflict A and Conflict B.

Conflict D — Overscan in rows, not pixels (small but real)

The v3 codebase already has a comment explaining this:

src/components/diff-surface/v3/v3HeightConstants.ts:113-118

/**
 * Virtualizer overscan (rows above + below the viewport). TanStack measures
 * overscan in *rows*, not pixels — but v3 has many 0-px rows (hunk anchors,
 * collapsed files), which steal items from the budget without contributing
 * pixel buffer. We keep this small and rely on the pixel-budget rangeExtractor
 * (`PIXEL_OVERSCAN`) for render-ahead. Tiny base buffer here just avoids
 * one-frame edge flickers between rangeExtractor recomputes.
 */

displayMap interleaves real code rows with zero-height items: hunk anchors are 0 px, entire collapsed files are 0 px. A row-counting overscan can spend its whole budget on collapsed files and render zero pixels of buffer. v2 worked around this by injecting a custom rangeExtractor; v3 sidesteps it by working directly in pixel space against displayOffsets, where 0-px items are naturally invisible to the visibility computation:

src/components/diff-surface/v3/render/RowRenderer.ts:177-186 (visibility test in pixels)

const item = displayToItem(displayMap, d);
const top = offsets[d] ?? 0;
const next = offsets[d + 1] ?? top;
const height = next - top;
if (height <= 0) continue;
if (top >= visibleEndPx) continue;
if (next <= visibleStartPx) continue;

Workable through TanStack, but it's another seam where the library's defaults fight v3's data shape.

Conflict E — The 1Mpx layer-cap problem (transform-scroll decoupling)

TanStack Virtual sizes its scroll element to the total content height. Both Chromium and WebKit start to misbehave with single elements taller than ~1,000,000 px — scrollTop becomes nonlinear, transform: translate3d(0, -y, 0) aliases at extreme y. Monaco caps at exactly this value; v3 adopts the cap and decouples logical scroll from DOM scrollTop:

src/components/diff-surface/v3/render/useTransformScroll.ts:9-26

/**
 * Largest content height we will ever ask the browser to lay out as a single
 * positioned element. Above this, WebKit (and Chromium, less aggressively)
 * starts to refuse to allocate a single composited layer; scroll math becomes
 * non-linear; and `transform: translate3d(0, -y, 0)` for very large `y`
 * begins to alias / round to integer pixels strangely.
 *
 * Monaco caps at exactly this value. We adopt the same.
 *
 * Below the cap: `proxyScrollTop === logicalScrollTop`. They map 1:1.
 *
 * Above the cap: the proxy element's `scrollHeight` is clamped to
 * `MAX_LAYER_HEIGHT_PX`, but `logicalScrollTop` may exceed `proxyMax`.
 * We map the proxy's `[0, proxyMax]` linearly onto `[0, logicalMax]`. This
 * compresses one proxy pixel into multiple logical rows — scrolling becomes
 * coarser at extreme content heights, which is acceptable and matches Monaco.
 */
export const MAX_LAYER_HEIGHT_PX = 1_000_000;

50K rows × 20 px = 1Mpx; the cap is reachable for normal-sized diffs in this app. TanStack Virtual has no notion of a logical/proxy split; it would happily try to size the scroll element at 8Mpx for a 400K-row diff and inherit the platform's layer-cap quirks. You can wrap TanStack inside a transform-scroll shim, but again you're paying the library tax for less than what we already have.

Conflict F — Slot insertion shifts offsets after the fact (revisits Law 1)

Slots in v3 — composer, threads, hunk-expansion — are interleaved into the row stream by displayItems.ts and pixel-positioned in one prefix sum. Each slot's height is set by its producer from data, never measured:

src/components/diff-surface/v3/displayItems.ts:33-40

/**
 * Pixel height of the slot. Set by the slot's producer at construction
 * time; v3 does NOT measure rendered slot DOM. This must be deterministic
 * from the slot's data so `estimateSize` is exact and `scrollToIndex`
 * lands on first call (no correction loop).
 */
readonly height: number;

In a TanStack-backed surface, slots become "items the virtualizer wants to measure when they mount," which retriggers the same shift-after-mount class of bug v3 is built to eliminate. You can disable measurement and go pure-estimateSize, but the slot's producer must then own height computation deterministically — which is exactly what v3 already does, without the library wrapper.


3. What you'd have to do to make TanStack Virtual work, and what it costs

The hypothetical "TanStack Virtual–backed v3" is constructible. Roughly:

  1. Use useVirtualizer with count = displayMap.displayCount.
  2. estimateSize: i => offsets[i + 1] - offsets[i] (so heights are still data-driven).
  3. Never call measureElement. Set shouldAdjustScrollPositionOnItemSizeChange = () => false defensively.
  4. Replace TanStack's auto-rendering with: in a useLayoutEffect keyed on virtualizer.range, call imperativeRowRenderer.update(...) — i.e. extract the visible window from TanStack and feed it to the imperative renderer.
  5. Wrap the surface in React.memo and aggressive useMemo to suppress re-renders above the virtualizer.
  6. Layer transform-scroll on top to handle the 1Mpx cap.

The cost ledger:

Property Native v3 TanStack Virtual–backed v3
React work per scroll frame Zero. One style.transform write + one RowRenderer.update. Non-zero. The component holding useVirtualizer re-renders per scroll frame even if it returns the same JSX shape.
Mounted DOM Recycled imperative rows; rowsMountedNow < ~100. Same, if you skip TanStack's default rendering and call RowRenderer.update from a layout effect. (See bullet 4 above.)
Per-row token rendering buildRowHtml string + recycle gate. Same, by the same workaround.
Slot / hunk / collapsed-file heights Data-driven, deterministic. Same, if you commit to never calling measureElement.
Overscan budget Pixel-based via prefix sum. Row-based by default; needs a custom rangeExtractor.
1Mpx layer cap Handled in useTransformScroll. Needs an external transform-scroll shim around TanStack.
Lines of code added useTransformScroll (157) + RowRenderer (441) + displayOffsets (52) + displayIndexAtPx (33) = ~683 LOC of focused, tested code (wc -l on the v3 render module). The same ~683 LOC, plus the @tanstack/react-virtual dependency (currently ^3.13.24, package.json:68), plus a glue layer to bypass each of TanStack's six default behaviors above.

Net: the cost of using TanStack Virtual is roughly strictly more code than not using it, because every property v3 cares about is one TanStack opts out of by default. The library is paying for behaviors v3 forbids.


4. Where TanStack Virtual is the right choice in this codebase

Three things would be wrong to read into the above:

(a) That TanStack Virtual is bad. It's not. It's the right default for "list of React components, heights ranging from semi-dynamic to dynamic, you want a maintained library to handle the windowing." The Copilot Tauri repo uses it exactly this way in three places that are all reasonable:

(b) That v3 should refactor TanStack out of those other surfaces. No — they aren't subject to v3's complexity contract.

(c) That a future v3 component (file-tree sidebar, find-results panel, etc.) shouldn't use TanStack Virtual. Those should use it. The contract is specifically about the diff surface's hot paths, not "no library in the codebase ever."

The decision rule that comes out of this analysis:

Use @tanstack/react-virtual when (1) rows are React components, (2) heights are simple or you can absorb a one-frame measurement loop, and (3) per-frame React reconciliation over the visible window is acceptable. Use the imperative RowRenderer + useTransformScroll pattern when any of those breaks down — which, on the diff surface, all three do.


5. TL;DR

  • v2 was TanStack Virtual. v3 is the deliberate exit.
  • Five of TanStack's defaults conflict with v3's hard laws: per-frame React render, per-row React component rendering, ResizeObserver-based dynamic measurement, row-based overscan, no transform-scroll decoupling.
  • You can defeat each of those individually — but doing so leaves you with the existing v3 code wrapped in a library that has stopped earning its keep.
  • The cost of using TanStack Virtual on this surface is more code, not less, because every behavior that motivates the library is one v3 has to opt out of.
  • TanStack Virtual remains the right choice for chat lists, PR-patch viewers, sidebars, and any list of React components — and the codebase already uses it for exactly those.

If a future TanStack Virtual release ships:

  1. A mode: "controlled" API where heights, offsets, and the visible-window computation are owned by the consumer; AND
  2. A render: "imperative" API where the consumer calls into the virtualizer for a window and renders themselves with no per-scroll React work; AND
  3. A way to disable the internal scroll listener and feed in a logical scroll offset from outside,

— then it would be worth re-evaluating. As of v3.13 (current pinned version), none of these exist.


Appendix: Sources cited

Source Purpose
PR #4382 description v3 motivation, scope, follow-ups
docs/diff-surface-v3-architecture.md Why v3 exists; pillar walkthrough
docs/diff-surface-v3-plan.md TanStack risk register, decision history
docs/diff-surface-rebuild-plan.md Earlier rebuild context
skills/diff-surface-v3/SKILL.md The eight hard laws + anti-pattern catalogue
src/components/diff-surface/v3/render/RowRenderer.ts Imperative recycler, 441 LOC
src/components/diff-surface/v3/render/useTransformScroll.ts Logical/proxy scroll decoupling, 1Mpx cap
src/components/diff-surface/v3/render/DiffViewport.tsx React shell + imperative handle
src/components/diff-surface/v3/displayOffsets.ts Float64Array prefix-sum offsets
src/components/diff-surface/v3/displayItems.ts Display-index ↔ row/slot mapping
src/components/diff-surface/v3/v3HeightConstants.ts Single source of height truth; overscan note
src/components/diff-surface/v3/probes.ts surfaceRenderCount, rowsMountedNow, etc.
src/components/diff-surface/AllFilesDiffSurface.tsx v2: the existing TanStack-driven implementation
TanStack Virtual docs Library API, measureElement, dynamic-size guidance
package.json:68 Pinned @tanstack/react-virtual: ^3.13.24

Appendix: Caveats and uncertainty

  • The 1Mpx layer-cap claim cites Monaco's choice and v3's reproduced cap; Chromium's and WebKit's exact thresholds vary by version and platform. The point isn't the precise number — it's that TanStack Virtual has no decoupling layer to handle any such cap.
  • "Zero React render per scroll frame" assumes the host component above useVirtualizer is properly memoized. In a poorly-memoized host, the count is higher; in a well-memoized one, it's bounded but nonzero. v3 reaches actually-zero by going around React entirely for the transform write.
  • The "TanStack-backed v3 would be more code" estimate is qualitative. The author has not built and measured the hypothetical. If you are skeptical, the right experiment is to wire a TanStack-backed DiffViewport behind a feature flag and run the existing v3 perf probes (window.__diffSurfaceV3Probes) against a MEDIUM/LARGE fixture. The probes are designed exactly to make this comparison falsifiable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment