From de28e2a6eb4707210217ca8bed6637014347c4bf Mon Sep 17 00:00:00 2001 From: "Jason (Json)" <263060202+fuller-stack-dev@users.noreply.github.com> Date: Wed, 17 Jun 2026 21:58:02 -0600 Subject: [PATCH] fix: stabilize sparse skill category pagination (#2724) Merged after exact-head maintainer review. Prepared head SHA: 66d9662023fd3e3c36ceb541f11c14b98f9ed76b Autoreview: clean, no accepted/actionable findings. Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com> Reviewed-by: @fuller-stack-dev --- src/__tests__/skills-index.test.tsx | 91 ++++++++++++++++++++++ src/routes/skills/-SkillsResults.tsx | 4 +- src/routes/skills/-useSkillsBrowseModel.ts | 68 +++++++++++----- 3 files changed, 140 insertions(+), 23 deletions(-) diff --git a/src/__tests__/skills-index.test.tsx b/src/__tests__/skills-index.test.tsx index 1f10fb9a..e230d984 100644 --- a/src/__tests__/skills-index.test.tsx +++ b/src/__tests__/skills-index.test.tsx @@ -724,6 +724,97 @@ describe("SkillsIndex", () => { expect(screen.getByRole("button", { name: "Load more" })).toBeTruthy(); }); + it("keeps loading across empty filtered pages without flashing terminal states", async () => { + class IntersectionObserverMock { + observe = vi.fn(); + disconnect = vi.fn(); + } + vi.stubGlobal( + "IntersectionObserver", + IntersectionObserverMock as unknown as typeof IntersectionObserver, + ); + searchMock = { category: "automation" }; + convexHttpMock.query + .mockResolvedValueOnce({ + page: [], + hasMore: true, + nextCursor: "cursor-1", + }) + .mockReturnValueOnce(new Promise(() => {})); + + render(); + await act(async () => {}); + + expect(convexHttpMock.query).toHaveBeenCalledTimes(2); + expect(getLastListPageArgs()).toEqual(expect.objectContaining({ cursor: "cursor-1" })); + expect(screen.getByRole("status", { name: "Loading results" })).toBeTruthy(); + expect(screen.queryByText("Scroll to load more")).toBeNull(); + expect(screen.queryByText("No skills found")).toBeNull(); + }); + + it("bounds empty filtered page auto-advance and pauses for a manual retry", async () => { + class IntersectionObserverMock { + observe = vi.fn(); + disconnect = vi.fn(); + } + vi.stubGlobal( + "IntersectionObserver", + IntersectionObserverMock as unknown as typeof IntersectionObserver, + ); + searchMock = { category: "automation" }; + convexHttpMock.query + .mockResolvedValueOnce({ + page: [], + hasMore: true, + nextCursor: "cursor-1", + }) + .mockResolvedValueOnce({ + page: [], + hasMore: true, + nextCursor: "cursor-2", + }) + .mockResolvedValueOnce({ + page: [], + hasMore: true, + nextCursor: "cursor-3", + }) + .mockReturnValueOnce(new Promise(() => {})); + + render(); + await act(async () => {}); + + expect(convexHttpMock.query).toHaveBeenCalledTimes(3); + expect(screen.getByRole("button", { name: "Load more" })).toBeTruthy(); + expect(screen.queryByText("Scroll to load more")).toBeNull(); + expect(screen.queryByText("No skills found")).toBeNull(); + }); + + it("keeps the retry cursor when a filtered follow-up page fails", async () => { + vi.stubGlobal("IntersectionObserver", undefined); + vi.spyOn(console, "error").mockImplementation(() => {}); + searchMock = { category: "automation" }; + convexHttpMock.query + .mockResolvedValueOnce({ + page: [], + hasMore: true, + nextCursor: "cursor-1", + }) + .mockRejectedValueOnce(new Error("temporary failure")) + .mockReturnValueOnce(new Promise(() => {})); + + render(); + await act(async () => {}); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Load more" })); + }); + + expect(convexHttpMock.query).toHaveBeenCalledTimes(3); + expect(getLastListPageArgs()).toEqual(expect.objectContaining({ cursor: "cursor-1" })); + expect(screen.getByRole("status", { name: "Loading results" })).toBeTruthy(); + expect(screen.queryByText("No skills found")).toBeNull(); + }); + it("shows skeletons during load-more", async () => { vi.stubGlobal("IntersectionObserver", undefined); convexHttpMock.query diff --git a/src/routes/skills/-SkillsResults.tsx b/src/routes/skills/-SkillsResults.tsx index 0e321974..eedb02d7 100644 --- a/src/routes/skills/-SkillsResults.tsx +++ b/src/routes/skills/-SkillsResults.tsx @@ -28,7 +28,7 @@ export function SkillsResults({ isLoadingSkills, sorted, view, - listDoneLoading: _listDoneLoading, + listDoneLoading, hasQuery, canLoadMore, isLoadingMore, @@ -40,7 +40,7 @@ export function SkillsResults({ <> {isLoadingSkills ? ( - ) : sorted.length === 0 ? ( + ) : sorted.length === 0 && listDoneLoading ? (

No skills found

diff --git a/src/routes/skills/-useSkillsBrowseModel.ts b/src/routes/skills/-useSkillsBrowseModel.ts index 056416d9..e3898e3f 100644 --- a/src/routes/skills/-useSkillsBrowseModel.ts +++ b/src/routes/skills/-useSkillsBrowseModel.ts @@ -12,6 +12,7 @@ import { parseDir, parseSort, toListSort, type SortDir, type SortKey } from "./- import type { SkillListEntry, SkillSearchEntry } from "./-types"; const pageSize = 25; +const maxConsecutiveEmptyPagesPerFetch = 3; function isNavigationAbortError(err: unknown) { if (!(err instanceof Error)) return false; @@ -96,35 +97,57 @@ export function useSkillsBrowseModel({ const [listResults, setListResults] = useState([]); const [listCursor, setListCursor] = useState(null); const [listStatus, setListStatus] = useState("loading"); + const [listAutoLoadPaused, setListAutoLoadPaused] = useState(false); const fetchGeneration = useRef(0); const fetchPage = useCallback( async (cursor: string | null, generation: number) => { + let pageCursor = cursor; + let consecutiveEmptyPages = 0; try { - const result = await convexHttp.query(api.skills.listPublicPageV4, { - cursor: cursor ?? undefined, - numItems: pageSize, - ...(listSort ? { sort: listSort } : {}), - dir, - highlightedOnly: featuredOnly, - categorySlug: activeCategory?.slug, - topic: activeTopic, - ...(activeCategory ? { officialFirst: true } : {}), - categoryKeywords, - excludeCategoryKeywords, - }); - if (generation !== fetchGeneration.current) return; - setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page)); - const canAdvance = result.hasMore && result.nextCursor != null; - setListCursor(canAdvance ? result.nextCursor : null); - setListStatus(canAdvance ? "idle" : "done"); + while (true) { + const result = await convexHttp.query(api.skills.listPublicPageV4, { + cursor: pageCursor ?? undefined, + numItems: pageSize, + ...(listSort ? { sort: listSort } : {}), + dir, + highlightedOnly: featuredOnly, + categorySlug: activeCategory?.slug, + topic: activeTopic, + ...(activeCategory ? { officialFirst: true } : {}), + categoryKeywords, + excludeCategoryKeywords, + }); + if (generation !== fetchGeneration.current) return; + const nextCursor = + result.hasMore && result.nextCursor != null && result.nextCursor !== pageCursor + ? result.nextCursor + : null; + + // Filtered scans can yield empty transport pages before reaching visible results. + if (result.page.length === 0 && nextCursor) { + consecutiveEmptyPages += 1; + if (consecutiveEmptyPages < maxConsecutiveEmptyPagesPerFetch) { + pageCursor = nextCursor; + continue; + } + } + + setListResults((prev) => (cursor ? [...prev, ...result.page] : result.page)); + setListCursor(nextCursor); + setListAutoLoadPaused(result.page.length === 0 && Boolean(nextCursor)); + setListStatus(nextCursor ? "idle" : "done"); + return; + } } catch (err) { if (generation !== fetchGeneration.current) return; if (!isNavigationAbortError(err)) { console.error("Failed to fetch skills page:", err); } // Reset to idle so the user can retry via "Load more" - setListStatus(cursor ? "idle" : "done"); + setListCursor(pageCursor); + setListAutoLoadPaused(Boolean(pageCursor)); + setListStatus(pageCursor ? "idle" : "done"); } }, [ @@ -147,6 +170,7 @@ export function useSkillsBrowseModel({ const generation = fetchGeneration.current; setListResults([]); setListCursor(null); + setListAutoLoadPaused(false); setListStatus("loading"); void fetchPage(null, generation); return () => { @@ -300,11 +324,13 @@ export function useSkillsBrowseModel({ ? !isSearching && searchResults.length === searchLimit && searchResults.length > 0 : canLoadMoreList; const isLoadingMore = hasQuery ? isSearching && searchResults.length > 0 : isLoadingMoreList; - const canAutoLoad = typeof IntersectionObserver !== "undefined"; + const canAutoLoad = + typeof IntersectionObserver !== "undefined" && (hasQuery || !listAutoLoadPaused); const loadMore = useCallback(() => { if (loadMoreInFlightRef.current || isLoadingMore || !canLoadMore) return; loadMoreInFlightRef.current = true; + setListAutoLoadPaused(false); if (hasQuery) { setSearchLimit((value) => value + pageSize); } else { @@ -320,7 +346,7 @@ export function useSkillsBrowseModel({ }, [isLoadingMore]); useEffect(() => { - if (!canLoadMore || typeof IntersectionObserver === "undefined") return () => {}; + if (!canLoadMore || !canAutoLoad) return () => {}; const target = loadMoreRef.current; if (!target) return () => {}; const observer = new IntersectionObserver( @@ -334,7 +360,7 @@ export function useSkillsBrowseModel({ ); observer.observe(target); return () => observer.disconnect(); - }, [canLoadMore, loadMore]); + }, [canAutoLoad, canLoadMore, loadMore]); useEffect(() => { return () => window.clearTimeout(navigateTimer.current);