refactor(feed): cap the read-ahead lookahead and extract it to a tested helper
Review follow-up. The viewport-scaled lookahead was unbounded, so a tall window prefetched several feed pages on every mount / filter change before any scroll — wasted work on the app's heaviest query. Clamp it to MAX_LOOKAHEAD_ROWS, and move the policy into a pure readAhead.ts (readAheadLookahead / shouldPrefetchNext) with unit tests covering the floor, the scaling, the cap boundary, and the fetch trigger — the core of the feed-scroll fix now has coverage instead of only manual E2E.
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
||||
type ReactNode,
|
||||
} from "react";
|
||||
import { useVirtualizer } from "@tanstack/react-virtual";
|
||||
import { shouldPrefetchNext } from "./readAhead";
|
||||
|
||||
// The virtualization engine, lifted out of VirtualFeed so any long client- or server-side list can
|
||||
// reuse it (the feed's video grid, the channel manager's card/row views). It owns ONLY the layout
|
||||
@@ -23,17 +24,6 @@ import { useVirtualizer } from "@tanstack/react-virtual";
|
||||
// (grid-cols-[repeat(auto-fill,minmax(260px,1fr))] with a gap-4 / 1rem gutter).
|
||||
const GRID_GAP = 16;
|
||||
const LIST_GAP = 4;
|
||||
// Infinite-scroll read-ahead. The next page is pulled once the last rendered row comes within
|
||||
// `lookahead` rows of the loaded end, where lookahead = max(PREFETCH_ROWS, rendered-rows ×
|
||||
// PREFETCH_VIEWPORTS). Sizing it to the number of rows the viewport currently renders makes it adapt
|
||||
// to window height / column count / row height, and keeps ~PREFETCH_VIEWPORTS screenfuls buffered
|
||||
// below the fold. The fixed floor is the finite-list / first-paint case.
|
||||
const PREFETCH_ROWS = 4;
|
||||
// Keep this many screenfuls of rows loaded ahead of the fold. A fast free-spin flick (MX Master &
|
||||
// co.) out-scrolls a mere 4-row lookahead and lands on an empty bottom, then waits out serial keyset
|
||||
// round-trips — the "Load more" hangs for seconds. Buffering a couple of screens ahead means the
|
||||
// content is usually already there when the user gets to it.
|
||||
const PREFETCH_VIEWPORTS = 2;
|
||||
|
||||
/** Columns that fit `width`, or 1 for the single-column layout (`minCol == null`). */
|
||||
function columnsFor(minCol: number | null, width: number): number {
|
||||
@@ -148,17 +138,17 @@ export default function VirtualGrid<T>({
|
||||
|
||||
const virtualRows = virtualizer.getVirtualItems();
|
||||
|
||||
// Infinite-scroll read-ahead: once the last rendered row is within `lookahead` rows of the end,
|
||||
// pull the next page. A no-op for finite lists (no fetchNextPage / hasNextPage false). Because this
|
||||
// effect re-runs when a page settles (isFetchingNextPage flips false) it CHAINS — refilling the
|
||||
// buffer page by page as the user consumes it, so a fast scroll reaches the loaded end far less
|
||||
// often. See PREFETCH_VIEWPORTS above for why the lookahead scales with the rendered row count.
|
||||
// Infinite-scroll read-ahead: pull the next page once the last rendered row is within the
|
||||
// read-ahead window of the loaded end (see readAhead.ts — the window scales with the rendered row
|
||||
// count, clamped so a tall window doesn't prefetch unboundedly). A no-op for finite lists (no
|
||||
// fetchNextPage / hasNextPage false). Because this effect re-runs when a page settles
|
||||
// (isFetchingNextPage flips false) it CHAINS — refilling the buffer as the user consumes it, so a
|
||||
// fast scroll reaches the loaded end far less often.
|
||||
useEffect(() => {
|
||||
if (!fetchNextPage || !hasNextPage || isFetchingNextPage) return;
|
||||
const last = virtualRows[virtualRows.length - 1];
|
||||
if (!last) return;
|
||||
const lookahead = Math.max(PREFETCH_ROWS, virtualRows.length * PREFETCH_VIEWPORTS);
|
||||
if (last.index >= rows.length - 1 - lookahead) {
|
||||
if (shouldPrefetchNext(last.index, rows.length, virtualRows.length)) {
|
||||
fetchNextPage();
|
||||
}
|
||||
}, [virtualRows, rows.length, hasNextPage, isFetchingNextPage, fetchNextPage]);
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
MAX_LOOKAHEAD_ROWS,
|
||||
PREFETCH_ROWS,
|
||||
PREFETCH_VIEWPORTS,
|
||||
readAheadLookahead,
|
||||
shouldPrefetchNext,
|
||||
} from "./readAhead";
|
||||
|
||||
describe("readAheadLookahead", () => {
|
||||
it("floors at PREFETCH_ROWS when nothing (or little) is rendered", () => {
|
||||
expect(readAheadLookahead(0)).toBe(PREFETCH_ROWS);
|
||||
expect(readAheadLookahead(1)).toBe(PREFETCH_ROWS); // 1*2=2 < floor 4
|
||||
});
|
||||
|
||||
it("scales with the rendered row count for a normal viewport", () => {
|
||||
expect(readAheadLookahead(10)).toBe(10 * PREFETCH_VIEWPORTS); // 20
|
||||
expect(readAheadLookahead(15)).toBe(30);
|
||||
});
|
||||
|
||||
it("caps at MAX_LOOKAHEAD_ROWS so a tall window doesn't prefetch unboundedly", () => {
|
||||
// 30 rendered rows would be 60 uncapped — the cap is what bounds per-mount feed prefetch.
|
||||
expect(readAheadLookahead(30)).toBe(MAX_LOOKAHEAD_ROWS);
|
||||
expect(readAheadLookahead(1000)).toBe(MAX_LOOKAHEAD_ROWS);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldPrefetchNext", () => {
|
||||
it("fetches once the last rendered row enters the read-ahead window", () => {
|
||||
// 100 rows loaded, viewport renders 10 → lookahead 20. Fetch when lastIndex >= 100-1-20 = 79.
|
||||
expect(shouldPrefetchNext(78, 100, 10)).toBe(false);
|
||||
expect(shouldPrefetchNext(79, 100, 10)).toBe(true);
|
||||
expect(shouldPrefetchNext(99, 100, 10)).toBe(true);
|
||||
});
|
||||
|
||||
it("fetches eagerly while fewer rows are loaded than the lookahead (early pages)", () => {
|
||||
// Only 15 rows loaded, lookahead 20 → the whole list is inside the window.
|
||||
expect(shouldPrefetchNext(8, 15, 10)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not fetch near the top of a long loaded list", () => {
|
||||
expect(shouldPrefetchNext(12, 500, 10)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,30 @@
|
||||
// Infinite-scroll read-ahead policy for VirtualGrid, extracted as a pure function so the decision is
|
||||
// unit-testable without a layout engine.
|
||||
//
|
||||
// The next page is pulled once the last rendered row comes within `lookahead` rows of the loaded end,
|
||||
// where lookahead scales with the number of rows the viewport currently renders (so it adapts to
|
||||
// window height / column count / row height) but is clamped to a floor and a ceiling:
|
||||
// - floor PREFETCH_ROWS: the finite-list / first-paint case where nothing is rendered yet.
|
||||
// - ceiling MAX_LOOKAHEAD_ROWS: a tall window renders many rows, and an uncapped lookahead would
|
||||
// eagerly prefetch several pages on every mount / filter change — wasted work on the app's
|
||||
// heaviest query (the feed). The cap bounds that while still buffering a couple of screens ahead.
|
||||
export const PREFETCH_ROWS = 4;
|
||||
export const PREFETCH_VIEWPORTS = 2;
|
||||
export const MAX_LOOKAHEAD_ROWS = 40;
|
||||
|
||||
/** How many rows ahead of the last rendered row to keep buffered, for a viewport rendering
|
||||
* `renderedRows` rows. Clamped to [PREFETCH_ROWS, MAX_LOOKAHEAD_ROWS]. */
|
||||
export function readAheadLookahead(renderedRows: number): number {
|
||||
return Math.min(MAX_LOOKAHEAD_ROWS, Math.max(PREFETCH_ROWS, renderedRows * PREFETCH_VIEWPORTS));
|
||||
}
|
||||
|
||||
/** Whether to fetch the next page: true once the last rendered row (`lastIndex`) is within the
|
||||
* read-ahead window of the loaded end (`rowCount` total loaded rows). Because the caller re-runs
|
||||
* this as each page settles, it chains — refilling the buffer as the user consumes it. */
|
||||
export function shouldPrefetchNext(
|
||||
lastIndex: number,
|
||||
rowCount: number,
|
||||
renderedRows: number,
|
||||
): boolean {
|
||||
return lastIndex >= rowCount - 1 - readAheadLookahead(renderedRows);
|
||||
}
|
||||
Reference in New Issue
Block a user