perf(ui): a next-page fetch joins the one in flight instead of sending the cursor again
react-query's fetchNextPage cancels a next page still loading and sends it again by default.
Our requests ignore the abort signal, so the server ran both: the feed's read-ahead effect
re-runs on scroll frames before isFetchingNextPage reaches it, and on prod every feed page
went out 2-3 times (a 0.8 s title sort queued to 2.3 s). Every call now passes NEXT_PAGE
({cancelRefetch: false}), lint-enforced, with a contract test.
This commit is contained in:
@@ -547,6 +547,15 @@ export default tseslint.config(
|
||||
// lib/*.contract.test.ts).
|
||||
"no-restricted-syntax": [
|
||||
"error",
|
||||
// A next-page fetch called bare CANCELS the one in flight and sends the same cursor again;
|
||||
// our requests ignore the abort, so the server ran both (E31 S4e: every feed page 2–3×).
|
||||
// lib/nextPage.ts NEXT_PAGE makes the second call join the first.
|
||||
{
|
||||
selector:
|
||||
"CallExpression[arguments.length=0]:matches([callee.name='fetchNextPage'], [callee.property.name='fetchNextPage'])",
|
||||
message:
|
||||
"fetchNextPage() cancels and re-sends a page already loading: call fetchNextPage(NEXT_PAGE) (lib/nextPage.ts).",
|
||||
},
|
||||
// `enabled:` sees only the native attribute; a FocusSafeButton that holds the focus is
|
||||
// disabled by aria-disabled, so a bare `enabled:hover:` lit it up on hover as if it worked.
|
||||
{
|
||||
|
||||
@@ -13,6 +13,7 @@ import { api, type LogFacet, type LogQuery, type ServerLogRow } from "../lib/api
|
||||
import { copyText } from "../lib/clipboard";
|
||||
import { formatDateTimeSec } from "../lib/format";
|
||||
import { nextNavId, notifyNav, rememberNavPosition } from "../lib/history";
|
||||
import { NEXT_PAGE } from "../lib/nextPage";
|
||||
import { LEVEL_GROUPS, type LevelGroup } from "../lib/log";
|
||||
import {
|
||||
DEFAULT_LOGS_STATE,
|
||||
@@ -156,7 +157,7 @@ export default function AdminLogs() {
|
||||
? "countsFailed"
|
||||
: null;
|
||||
const retryFailed = () => {
|
||||
if (logs.isFetchNextPageError) void logs.fetchNextPage();
|
||||
if (logs.isFetchNextPageError) void logs.fetchNextPage(NEXT_PAGE);
|
||||
else if (logs.isError) void logs.refetch();
|
||||
if (facets.isError) void facets.refetch();
|
||||
};
|
||||
@@ -190,7 +191,7 @@ export default function AdminLogs() {
|
||||
|
||||
const loadOlder = () => {
|
||||
setLive(false);
|
||||
void logs.fetchNextPage();
|
||||
void logs.fetchNextPage(NEXT_PAGE);
|
||||
};
|
||||
|
||||
const copy = async () => {
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
useQueryClient,
|
||||
} from "@tanstack/react-query";
|
||||
import { qk } from "../lib/queryKeys";
|
||||
import { NEXT_PAGE } from "../lib/nextPage";
|
||||
import {
|
||||
ArrowLeft,
|
||||
Columns2,
|
||||
@@ -556,7 +557,7 @@ export default function Feed({
|
||||
<div className="text-center pt-4">
|
||||
<Button
|
||||
variant="outline"
|
||||
onClick={() => ytQuery.fetchNextPage()}
|
||||
onClick={() => void ytQuery.fetchNextPage(NEXT_PAGE)}
|
||||
loading={ytQuery.isFetchingNextPage}
|
||||
>
|
||||
{ytQuery.isFetchingNextPage
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "@tanstack/react-query";
|
||||
import { User, X } from "lucide-react";
|
||||
import { qk } from "../lib/queryKeys";
|
||||
import { NEXT_PAGE } from "../lib/nextPage";
|
||||
import { healPlexField } from "../lib/plexSearch";
|
||||
import { healPlexSort, plexSortOptions, PLEX_SORT_FALLBACK } from "../lib/plexSort";
|
||||
import type { SortState } from "../lib/columnSort";
|
||||
@@ -321,7 +322,8 @@ export default function PlexBrowse() {
|
||||
if (!sentinelEl) return;
|
||||
const io = new IntersectionObserver(
|
||||
(entries) => {
|
||||
if (entries[0]?.isIntersecting && hasNextPage && !isFetchingNextPage) fetchNextPage();
|
||||
if (entries[0]?.isIntersecting && hasNextPage && !isFetchingNextPage)
|
||||
void fetchNextPage(NEXT_PAGE);
|
||||
},
|
||||
{ rootMargin: "600px" },
|
||||
);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { Video, VideoStatus } from "../lib/api";
|
||||
import { FEED_VIEW_SPEC, type FeedView } from "../lib/feedView";
|
||||
import type { NEXT_PAGE } from "../lib/nextPage";
|
||||
import VideoCard from "./VideoCard";
|
||||
import { VideoMenu } from "./VideoMenu";
|
||||
import { useContextMenu } from "./ui/useContextMenu";
|
||||
@@ -29,7 +30,7 @@ export interface VirtualFeedProps {
|
||||
onOpen: (v: Video, startAt?: number | null) => void;
|
||||
hasNextPage: boolean;
|
||||
isFetchingNextPage: boolean;
|
||||
fetchNextPage: () => void;
|
||||
fetchNextPage: (options: typeof NEXT_PAGE) => unknown;
|
||||
}
|
||||
|
||||
// The feed's videos on the shared VirtualGrid engine — this wrapper only maps a FeedView to the
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
import { useVirtualizer } from "@tanstack/react-virtual";
|
||||
import { useListNav } from "../../lib/listNav";
|
||||
import { scrollParent } from "../../lib/listPaging";
|
||||
import { NEXT_PAGE } from "../../lib/nextPage";
|
||||
import { GRID_GAP, columnsFor, itemWidthFor } from "./gridColumns";
|
||||
import { shouldPrefetchNext } from "./readAhead";
|
||||
|
||||
@@ -66,7 +67,8 @@ export interface VirtualGridProps<T> {
|
||||
/** Infinite scroll — omit all three for a finite client-side list. */
|
||||
hasNextPage?: boolean;
|
||||
isFetchingNextPage?: boolean;
|
||||
fetchNextPage?: () => void;
|
||||
/** Called with `NEXT_PAGE`, so a read-ahead that fires again joins the page already loading. */
|
||||
fetchNextPage?: (options: typeof NEXT_PAGE) => unknown;
|
||||
}
|
||||
|
||||
export default function VirtualGrid<T>({
|
||||
@@ -167,7 +169,7 @@ export default function VirtualGrid<T>({
|
||||
const last = virtualRows[virtualRows.length - 1];
|
||||
if (!last) return;
|
||||
if (shouldPrefetchNext(last.index, rows.length, virtualRows.length)) {
|
||||
fetchNextPage();
|
||||
fetchNextPage(NEXT_PAGE);
|
||||
}
|
||||
}, [virtualRows, rows.length, hasNextPage, isFetchingNextPage, fetchNextPage]);
|
||||
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { eslint } from "./lintProbe.testkit";
|
||||
|
||||
// eslint.config.js: a bare `fetchNextPage()` cancels the page already loading and sends it again
|
||||
// (E31 S4e). Pins that the rule keeping every call on NEXT_PAGE actually fires.
|
||||
const MESSAGE = "fetchNextPage() cancels";
|
||||
|
||||
async function hits(source: string): Promise<number> {
|
||||
const [result] = await eslint.lintText(source, { filePath: "src/components/__probe.tsx" });
|
||||
return (result?.messages ?? []).filter((m) => m.message.startsWith(MESSAGE)).length;
|
||||
}
|
||||
|
||||
describe("a next-page fetch", () => {
|
||||
it("is refused bare, on the query or destructured", async () => {
|
||||
expect(
|
||||
await hits("export const a = (q: { fetchNextPage(): void }) => q.fetchNextPage();\n"),
|
||||
).toBe(1);
|
||||
expect(await hits("export const b = (fetchNextPage: () => void) => fetchNextPage();\n")).toBe(
|
||||
1,
|
||||
);
|
||||
});
|
||||
|
||||
it("passes with NEXT_PAGE", async () => {
|
||||
expect(
|
||||
await hits(
|
||||
'import { NEXT_PAGE } from "../lib/nextPage";\nexport const c = (q: { fetchNextPage(o: unknown): void }) => q.fetchNextPage(NEXT_PAGE);\n',
|
||||
),
|
||||
).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,35 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// A next-page call while that page is still loading joins it instead of sending it again (E31 S4e):
|
||||
// on prod every feed page went out 2–3 times with the same cursor.
|
||||
import { InfiniteQueryObserver } from "@tanstack/react-query";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { NEXT_PAGE } from "./nextPage";
|
||||
import { createQueryClient } from "./queryClient";
|
||||
|
||||
describe("NEXT_PAGE", () => {
|
||||
it("sends each page once when the read-ahead calls twice in a row", async () => {
|
||||
const sent: (string | null)[] = [];
|
||||
const observer = new InfiniteQueryObserver(createQueryClient(), {
|
||||
queryKey: ["probe"],
|
||||
initialPageParam: null as string | null,
|
||||
queryFn: async ({ pageParam }) => {
|
||||
sent.push(pageParam);
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
return { next: `c${sent.length}` };
|
||||
},
|
||||
getNextPageParam: (last: { next: string }) => last.next,
|
||||
});
|
||||
const unsubscribe = observer.subscribe(() => {});
|
||||
await observer.refetch();
|
||||
|
||||
// Two scroll frames before `isFetchingNextPage` reaches the component.
|
||||
void observer.fetchNextPage(NEXT_PAGE);
|
||||
await observer.fetchNextPage(NEXT_PAGE);
|
||||
|
||||
expect(sent).toEqual([null, "c1"]);
|
||||
expect(observer.getCurrentResult().data?.pages).toHaveLength(2);
|
||||
unsubscribe();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,9 @@
|
||||
/**
|
||||
* What every `fetchNextPage` call passes (lint-enforced). By default react-query CANCELS a next-page
|
||||
* fetch that is still in flight and sends it again with the same cursor. Our requests do not take the
|
||||
* abort signal, so the first one is not stopped: the server ran both. The feed's read-ahead effect
|
||||
* re-runs on every scroll frame, and a frame that lands before `isFetchingNextPage` turns true calls
|
||||
* again. On prod (E31 S4e) every feed page went out 2–3 times; on a 0.8 s title sort the copies
|
||||
* queued to 2.3 s. With this a second call joins the fetch already running.
|
||||
*/
|
||||
export const NEXT_PAGE = { cancelRefetch: false } as const;
|
||||
Reference in New Issue
Block a user