From 7c00db2b845a139dd28b2fd790ddbe8d02008958 Mon Sep 17 00:00:00 2001 From: khannurien Date: Sat, 10 Oct 2026 07:43:28 +0000 Subject: [PATCH] =?UTF-8?q?v4:=20what=20comes=20into=20a=20live=20list=20g?= =?UTF-8?q?oes=20where=20its=20date=20puts=20it=20=E2=80=94=20a=20dump=20e?= =?UTF-8?q?dited=20into=20a=20category=20stayed=20out=20of=20the=20index's?= =?UTF-8?q?=20feed=20of=20it=20until=20a=20reload,=20and=20a=20dump=20or?= =?UTF-8?q?=20a=20playlist=20made=20public=20went=20on=20top=20of=20the=20?= =?UTF-8?q?others'=20lists=20as=20if=20just=20posted.=20The=20index's=20fe?= =?UTF-8?q?ed=20now=20takes=20a=20dump=20edited=20into=20its=20scope,=20an?= =?UTF-8?q?d=20orders=20the=20dumps=20that=20arrive=20by=20date,=20as=20th?= =?UTF-8?q?e=20server=20pages=20them;=20a=20created=20playlist=20goes=20by?= =?UTF-8?q?=20date=20in=20the=20profile's=20and=20/playlists'=20lists,=20a?= =?UTF-8?q?=20new=20one=20first=20as=20before.=20Both=20use=20withByDate?= =?UTF-8?q?=20(utils/feed/pagedList.ts),=20which=20leaves=20an=20item=20ol?= =?UTF-8?q?der=20than=20every=20one=20loaded=20to=20a=20later=20page.=20Th?= =?UTF-8?q?e=20search=20runs=20again=20after=20a=20reconnection,=20for=20e?= =?UTF-8?q?very=20page=20of=20dumps=20loaded,=20with=20its=20users=20and?= =?UTF-8?q?=20playlists;=20it=20shares=20refetchPages=20with=20usePagedLis?= =?UTF-8?q?t's=20resync.=20injectDump's=20doc=20said=20a=20private=20dump?= =?UTF-8?q?=20is=20never=20broadcast,=20while=20its=20poster=20gets=20it.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- src/contexts/ws/WSContext.ts | 12 ++-- src/hooks/dump/useDumpListSync.ts | 20 ++----- src/hooks/feed/usePagedList.ts | 19 +----- src/hooks/playlist/usePlaylistListSync.ts | 15 ++--- src/pages/Index.tsx | 23 +++++--- src/pages/Search.tsx | 54 ++++++++++++++--- .../playlist/usePlaylistListSync.test.tsx | 10 ++++ src/test/pages/Index.test.tsx | 38 ++++++++++++ src/test/pages/Search.test.tsx | 46 +++++++++++++++ src/test/utils/feed/pagedList.test.ts | 59 +++++++++++++++++++ src/utils/feed/pagedList.ts | 43 ++++++++++++++ 11 files changed, 278 insertions(+), 61 deletions(-) diff --git a/src/contexts/ws/WSContext.ts b/src/contexts/ws/WSContext.ts index 992ca4a7..3e0460b9 100644 --- a/src/contexts/ws/WSContext.ts +++ b/src/contexts/ws/WSContext.ts @@ -210,10 +210,10 @@ export interface WSContextValue { /** As `myVotes`, for comments. */ myLikes: Set; /** - * Shows new dumps on the index as they are posted, above the feed `Index` - * fetched. It holds, newest first and at most 100, the ones the server - * announced this session, which are public, and the ones this user posted, - * which may be private. + * Shows new dumps on the index as they are posted, merged by date with the + * feed `Index` fetched. It holds, newest first and at most 100, the ones + * the server announced this session, which are public, and the ones this + * user posted, which may be private. */ recentDumps: Dump[]; /** @@ -258,8 +258,8 @@ export interface WSContextValue { removeCommentLike: (commentId: string, shownCount?: number) => void; /** * Shows a dump this user just posted on the index without waiting for its - * broadcast, which a private dump never gets. `DumpCreateModal` calls it, - * and it adds the dump to `recentDumps`. + * broadcast. `DumpCreateModal` calls it, and it adds the dump to + * `recentDumps`. */ injectDump: (dump: Dump) => void; /** diff --git a/src/hooks/dump/useDumpListSync.ts b/src/hooks/dump/useDumpListSync.ts index 07595921..dd0460b3 100644 --- a/src/hooks/dump/useDumpListSync.ts +++ b/src/hooks/dump/useDumpListSync.ts @@ -9,11 +9,11 @@ import { useEffect, useLayoutEffect, useRef } from "react"; import type { Dump } from "../../../shared/dump.ts"; import { useWS, useWSEvent } from "../ws/useWS.ts"; import { insertAt, usePositionMemory } from "../feed/usePositionMemory.ts"; +import { withByDate } from "../../utils/feed/pagedList.ts"; /** * Passed to {@link useDumpListSync} by the lists that need more than edits and - * deletions. The index's feed adds no dump through it, since it merges - * `WSContext`'s `recentDumps` itself. + * deletions. */ export interface DumpListSyncOptions { /** @@ -32,23 +32,11 @@ export interface DumpListSyncOptions { * Also adds a dump `belongs` accepts that the list never held, where the * newest-first order puts it among the dumps loaded: a new one first. One * older than all of them is left to the next pages. For the lists sorted by - * `createdAt`, as a profile's dumps. + * `createdAt`: the index's feed, a profile's dumps, the followed users'. */ addNew?: boolean; } -/** - * Returns `list` with `dump` before the first dump posted earlier, or as is - * when none was: the dump belongs to a later page. - */ -function insertByDate(list: Dump[], dump: Dump): Dump[] { - if (list.length === 0) return [dump]; - const idx = list.findIndex((d) => - d.createdAt.getTime() < dump.createdAt.getTime() - ); - return idx === -1 ? list : insertAt(list, dump, idx); -} - /** * Applies the socket's dump, comment and user events to `items`, the loaded * list, which `setDumps` updates. A deleted dump is removed, an edited one is @@ -96,7 +84,7 @@ export function useDumpListSync( } if (!accepted) return prev; if (oldIdx !== undefined) return insertAt(prev, dump, oldIdx); - return addNew ? insertByDate(prev, dump) : prev; + return addNew ? withByDate(prev, dump) : prev; }); }); diff --git a/src/hooks/feed/usePagedList.ts b/src/hooks/feed/usePagedList.ts index 02a2ac6a..d74ddc85 100644 --- a/src/hooks/feed/usePagedList.ts +++ b/src/hooks/feed/usePagedList.ts @@ -13,6 +13,7 @@ import { firstPage, mapItems, type PagedList, + refetchPages, restoredList, } from "../../utils/feed/pagedList.ts"; import type { FeedCacheEntry } from "./useFeedCache.ts"; @@ -114,22 +115,8 @@ export function usePagedList( const resync = useCallback(() => { const { list, fetchPage } = latestRef.current; if (!list) return; - const pages = Array.from({ length: list.page }, (_, i) => i + 1); - Promise.all(pages.map((page) => fetchPage(page))) - .then((results) => { - // An item added between two pages' fetches shifts the later page by - // one, which would list an item twice. - const byId = new Map(); - for (const item of results.flatMap((r) => r.items)) { - if (!byId.has(item.id)) byId.set(item.id, item); - } - const items = [...byId.values()]; - setData((l) => ({ - ...l, - items, - hasMore: results[results.length - 1].hasMore, - })); - }) + refetchPages(list.page, (page) => fetchPage(page)) + .then(({ items, hasMore }) => setData((l) => ({ ...l, items, hasMore }))) .catch(() => {}); }, [setData]); diff --git a/src/hooks/playlist/usePlaylistListSync.ts b/src/hooks/playlist/usePlaylistListSync.ts index 1c5c0548..e67b2b5e 100644 --- a/src/hooks/playlist/usePlaylistListSync.ts +++ b/src/hooks/playlist/usePlaylistListSync.ts @@ -10,6 +10,7 @@ import { useEffect, useLayoutEffect, useRef } from "react"; import type { Playlist } from "../../../shared/playlist.ts"; import { useWS, useWSEvent } from "../ws/useWS.ts"; import { insertAt, usePositionMemory } from "../feed/usePositionMemory.ts"; +import { withByDate } from "../../utils/feed/pagedList.ts"; /** Passed to {@link usePlaylistListSync} by each page, to fit its list. */ export interface PlaylistListSyncOptions { @@ -29,12 +30,12 @@ export interface PlaylistListSyncOptions { } /** - * Applies the socket's playlist events to `items`, the loaded list, which - * `setPlaylists` updates. A created playlist goes first, an updated one - * changes in place or leaves when it turns private (unless `isOwner`), one - * that left comes back where it was when it returns, a deleted one leaves, - * and a playlist's `dumpCount` follows its dumps. A renamed user's playlists - * take their new name. + * Applies the socket's playlist events to `items`, the loaded list, newest + * first, which `setPlaylists` updates. A created playlist goes where its date + * puts it, first when new, an updated one changes in place or leaves when it + * turns private (unless `isOwner`), one that left comes back where it was + * when it returns, a deleted one leaves, and a playlist's `dumpCount` follows + * its dumps. A renamed user's playlists take their new name. */ export function usePlaylistListSync( items: Playlist[], @@ -73,7 +74,7 @@ export function usePlaylistListSync( if (oldIdx !== undefined) return insertAt(prev, playlist, oldIdx); // An update never adds a playlist the list doesn't hold. if (ev.type === "updated" || noNewEntries) return prev; - return [playlist, ...prev]; + return withByDate(prev, playlist); } if (ev.type === "deleted") { diff --git a/src/pages/Index.tsx b/src/pages/Index.tsx index ba12cde4..04a8c602 100644 --- a/src/pages/Index.tsx +++ b/src/pages/Index.tsx @@ -55,7 +55,8 @@ export interface IndexProps { * user only. The feed comes from * `GET /api/dumps`, or `GET /api/dumps?category=`, a page at a time, * and the "hot", "new" and "journal" tabs share it. `WSContext` adds the dumps - * posted since, the live vote counts, and who's online. The router keys the + * posted since, or brought into the feed by an edit, by date, the live vote + * counts, and who's online. The router keys the * page by category, so switching tabs keeps the feed and switching categories * refetches it. It also opens the dump creation modal on a link shared from * Android (`?share_url=`). @@ -139,10 +140,7 @@ export function Index( () => dumpsState.status === "loaded" ? dumpsState.data.items : [], [dumpsState], ); - // Signed in, the feed holds the user's own private dumps. - useDumpListSync(dumps, setScopedItems, { isOwner: true }); - - // A category's feed takes only the new dumps filed under it. + // A category's feed takes only the dumps filed under it. const categoryId = category?.id; const inScope = useCallback( (d: Dump) => @@ -152,13 +150,24 @@ export function Index( [categorySlug, categoryId], ); + // Signed in, the feed holds the user's own private dumps. A dump edited + // into the category, or made public, comes in where its date puts it. + useDumpListSync(dumps, setScopedItems, { + isOwner: true, + belongs: inScope, + addNew: true, + }); + const restIds = useMemo(() => new Set(dumps.map((d) => d.id)), [dumps]); // Dumps deleted since loading are dropped, and the one the edit page just - // deleted before its `dump_deleted` arrives. + // deleted before its `dump_deleted` arrives. In the server's order, by date: + // a dump made public long after it was posted isn't news, and goes below + // what came since, even when older than every dump loaded. const combined = useMemo( () => [...recentDumps.filter((d) => inScope(d) && !restIds.has(d.id)), ...dumps] - .filter((d) => !deletedDumpIds.has(d.id) && d.id !== justDeletedId), + .filter((d) => !deletedDumpIds.has(d.id) && d.id !== justDeletedId) + .sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime()), [recentDumps, restIds, dumps, deletedDumpIds, justDeletedId, inScope], ); diff --git a/src/pages/Search.tsx b/src/pages/Search.tsx index 89484c7e..3f94407d 100644 --- a/src/pages/Search.tsx +++ b/src/pages/Search.tsx @@ -6,7 +6,7 @@ * @module */ -import { useCallback } from "react"; +import { useCallback, useLayoutEffect, useRef } from "react"; import { Link, useSearchParams } from "react-router"; import { t } from "@lingui/core/macro"; import { Trans } from "@lingui/react/macro"; @@ -39,11 +39,13 @@ import { firstPage, type PagedList, pageUrl, + refetchPages, } from "../utils/feed/pagedList.ts"; import { useFetchState } from "../hooks/shared/useFetchState.ts"; import { useLoadMore } from "../hooks/feed/usePagedList.ts"; import { useDumpListSync } from "../hooks/dump/useDumpListSync.ts"; import { usePlaylistListSync } from "../hooks/playlist/usePlaylistListSync.ts"; +import { useOnReconnect } from "../hooks/ws/useWS.ts"; import { SEARCH_TABS, searchPath, @@ -64,6 +66,18 @@ interface Results { playlists: Playlist[]; } +/** Returns the first page of a search's answer, as `Search` holds it. */ +function toResults( + { dumps, users, playlists }: Wire, +): Results { + return { + dumps: firstPage(dumps.items.map(deserializeDump), dumps.hasMore), + dumpTotal: dumps.total ?? 0, + users: users.map(deserializePublicUser), + playlists: playlists.map(deserializePlaylist), + }; +} + /** * Names, for each tab a category narrows, the kind of its categories and the * parameter of `GET /api/search` that takes one. @@ -80,7 +94,8 @@ const TAB_CATEGORIES: Partial< * from `GET /api/search`, signed in when the viewer is, so they find their own * private dumps and playlists too. The dumps load a page at a time as the * "dumps" tab scrolls. A new `q` aborts the search under way. The socket keeps - * the dumps and playlists found live, as edited or deleted, but adds none. + * the dumps and playlists found live, as edited or deleted, but adds none; a + * reconnection runs the search again, for every page of dumps loaded. * * On the "dumps" and "playlists" tabs, a menu narrows the results to a * category of their kind, kept in the URL's `?category=`, which another tab @@ -113,13 +128,7 @@ export function Search(): React.JSX.Element { categorySlug ?? "" }` : undefined, - (signal) => - search(1, signal).then(({ dumps, users, playlists }) => ({ - dumps: firstPage(dumps.items.map(deserializeDump), dumps.hasMore), - dumpTotal: dumps.total ?? 0, - users: users.map(deserializePublicUser), - playlists: playlists.map(deserializePlaylist), - })), + (signal) => search(1, signal).then(toResults), ); const dumps = state.status === "loaded" ? state.data.dumps : null; @@ -159,6 +168,33 @@ export function Search(): React.JSX.Element { { isOwner: true, noNewEntries: true }, ); + // What changed while the socket was down: every page of dumps loaded is + // fetched again, and the users and playlists, which come with the first. + const searchRef = useRef(search); + useLayoutEffect(() => { + searchRef.current = search; + }); + useOnReconnect(() => { + if (state.status !== "loaded") return; + const asked = search; + let first: Wire | undefined; + refetchPages(state.data.dumps.page, (page) => + asked(page).then((res) => { + if (page === 1) first = res; + return { + items: res.dumps.items.map(deserializeDump), + hasMore: res.dumps.hasMore, + }; + })) + .then(({ items, hasMore }) => { + // The query changed meanwhile, and its own results are on screen. + if (!first || asked !== searchRef.current) return; + const fresh = toResults(first); + setData((d) => ({ ...fresh, dumps: { ...d.dumps, items, hasMore } })); + }) + .catch(() => {}); + }); + const sentinelRef = useInfiniteScroll( loadMore, tab === "dumps" && canLoadMore(dumps), diff --git a/src/test/hooks/playlist/usePlaylistListSync.test.tsx b/src/test/hooks/playlist/usePlaylistListSync.test.tsx index 8712af49..0839d3d2 100644 --- a/src/test/hooks/playlist/usePlaylistListSync.test.tsx +++ b/src/test/hooks/playlist/usePlaylistListSync.test.tsx @@ -33,6 +33,16 @@ describe("usePlaylistListSync", () => { expect(ids(items())).toEqual(["b", "a"]); }); + test("a playlist made public goes where its date puts it", () => { + const at = (id: string, day: number) => + makePlaylist({ id, createdAt: new Date(Date.UTC(2026, 0, day)) }); + const { send, items } = setup([at("c", 30), at("a", 10)]); + send("created", at("b", 20)); + // Older than every one loaded: a later page holds it. + send("created", at("old", 1)); + expect(ids(items())).toEqual(["c", "b", "a"]); + }); + test("created: filtered by owner, privacy and noNewEntries", () => { const p = makePlaylist({ id: "p", userId: "them" }); const byOwner = setup([], { ownerId: "me" }); diff --git a/src/test/pages/Index.test.tsx b/src/test/pages/Index.test.tsx index f55e737c..7af06bce 100644 --- a/src/test/pages/Index.test.tsx +++ b/src/test/pages/Index.test.tsx @@ -264,6 +264,44 @@ describe("Index feeds", () => { expect(screen.queryByText("A song")).not.toBeInTheDocument(); }); + test("a dump edited into the category comes in where its date puts it", async () => { + const at = (minutes: number) => new Date(Date.now() - minutes * 60_000); + const tagged = (id: string, title: string, minutes: number) => + makeDump({ id, title, categoryIds: ["c-music"], createdAt: at(minutes) }); + route( + "GET /api/dumps", + () => page([tagged("n", "Newer", 1), tagged("o", "Older", 30)]), + ); + const { socket } = setup("/music/new"); + await screen.findByText("Older"); + socket.receive({ + type: "dump_updated", + dump: wire(tagged("m", "Now music", 10)), + }); + expect(titles()).toEqual(["Newer", "Now music", "Older"]); + }); + + test("a dump made public long after it was posted goes by its date, not on top", async () => { + const at = (minutes: number) => new Date(Date.now() - minutes * 60_000); + route("GET /api/dumps", () => + page([ + makeDump({ id: "n", title: "Newer", createdAt: at(1) }), + makeDump({ id: "o", title: "Older", createdAt: at(30) }), + ])); + const { socket } = setup("/new"); + await screen.findByText("Older"); + // To everyone but its poster, it arrives as created. + socket.receive({ + type: "dump_created", + dump: wire(makeDump({ id: "m", title: "Unveiled", createdAt: at(10) })), + }); + socket.receive({ + type: "dump_created", + dump: wire(makeDump({ id: "a", title: "Ancient", createdAt: at(600) })), + }); + expect(titles()).toEqual(["Newer", "Unveiled", "Older", "Ancient"]); + }); + test("an unknown category is a 404", async () => { route("GET /api/dumps", () => fail("nope", 404)); setup("/nonsense/new"); diff --git a/src/test/pages/Search.test.tsx b/src/test/pages/Search.test.tsx index 095c8980..ac30e23d 100644 --- a/src/test/pages/Search.test.tsx +++ b/src/test/pages/Search.test.tsx @@ -238,4 +238,50 @@ describe("Search", () => { await user.click(screen.getByRole("button", { name: /Playlists/ })); expect(screen.queryByText("Cats")).not.toBeInTheDocument(); }); + + test("a reconnect runs the search again, for every page loaded", async () => { + route( + "GET /api/search", + (req) => + req.url.searchParams.get("page") === "2" + ? results({ dumps: [makeDump({ title: "Second page" })] }) + : results({ + dumps: [makeDump({ title: "First page" })], + hasMore: true, + users: [makeUser({ username: "catlover" })], + }), + ); + open("/search?q=cat"); + const socket = FakeWebSocket.last; + socket.welcome(); + await screen.findByText("First page"); + const sentinel = FakeIntersectionObserver.instances.findLast((o) => + o.elements.size > 0 + )!; + act(() => sentinel.report(true)); + await screen.findByText("Second page"); + + // Gone while the socket was down: an edit, a deletion, a new user. + route( + "GET /api/search", + (req) => + req.url.searchParams.get("page") === "2" + ? results({ dumps: [] }) + : results({ + dumps: [makeDump({ title: "First page, edited" })], + hasMore: true, + users: [ + makeUser({ username: "catlover" }), + makeUser({ username: "cat2" }), + ], + }), + ); + socket.drop(); + await act(() => new Promise((r) => setTimeout(r, 600))); + FakeWebSocket.last.welcome(); + expect(await screen.findByText("First page, edited")).toBeInTheDocument(); + expect(screen.queryByText("Second page")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Users (2)" })) + .toBeInTheDocument(); + }); }); diff --git a/src/test/utils/feed/pagedList.test.ts b/src/test/utils/feed/pagedList.test.ts index 057fc363..c7c64e79 100644 --- a/src/test/utils/feed/pagedList.test.ts +++ b/src/test/utils/feed/pagedList.test.ts @@ -5,7 +5,9 @@ import { pageUrl, readFirstPage, readPage, + refetchPages, restoredList, + withByDate, withFirst, } from "../../../utils/feed/pagedList.ts"; import { calls, fail, route } from "../../fetch.ts"; @@ -57,4 +59,61 @@ describe("pagedList", () => { const items = [a, b]; expect(withFirst(items, { id: "b" })).toBe(items); }); + + test("withByDate puts an item where its date falls, newest first", () => { + const at = (id: string, day: number) => ({ + id, + createdAt: new Date(Date.UTC(2026, 0, day)), + }); + const list = [at("c", 30), at("b", 20), at("a", 10)]; + const ids = (xs: { id: string }[]) => xs.map((x) => x.id); + expect(ids(withByDate(list, at("new", 31)))).toEqual([ + "new", + "c", + "b", + "a", + ]); + expect(ids(withByDate(list, at("mid", 25)))).toEqual([ + "c", + "mid", + "b", + "a", + ]); + // A tie goes first, as something new does. + expect(ids(withByDate(list, at("tie", 20)))).toEqual([ + "c", + "tie", + "b", + "a", + ]); + // Older than everything loaded: a later page holds it. + expect(withByDate(list, at("old", 1))).toBe(list); + expect(withByDate(list, list[1])).toBe(list); + expect(ids(withByDate([], at("only", 1)))).toEqual(["only"]); + }); + + test("refetchPages reads each page again, each item once", async () => { + const pages: Record = { + 1: [{ id: "new" }, { id: "a" }], + // Shifted by the new item: "a" is on both. + 2: [{ id: "a" }, { id: "b" }], + }; + const result = await refetchPages( + 2, + (n) => Promise.resolve({ items: pages[n], hasMore: n === 1 }), + ); + expect(result).toEqual({ + items: [{ id: "new" }, { id: "a" }, { id: "b" }], + hasMore: false, + }); + await expect( + refetchPages( + 2, + (n) => + n === 2 + ? Promise.reject(new Error("offline")) + : Promise.resolve({ items: [], hasMore: true }), + ), + ).rejects.toThrow("offline"); + }); }); diff --git a/src/utils/feed/pagedList.ts b/src/utils/feed/pagedList.ts index 2191adb9..1d92071e 100644 --- a/src/utils/feed/pagedList.ts +++ b/src/utils/feed/pagedList.ts @@ -103,3 +103,46 @@ export async function readFirstPage( export function withFirst(items: T[], item: T): T[] { return items.some((i) => i.id === item.id) ? items : [item, ...items]; } + +/** + * Returns `items`, newest first as the API pages them, with `item` where its + * `createdAt` puts it: before the first item no newer than it. An item older + * than every one loaded is left to a later page, and `items` returned as they + * are, as they are when `item` is already there. The list sync hooks put a + * new or newly visible dump or playlist in a list with it. + */ +export function withByDate( + items: T[], + item: T, +): T[] { + if (items.some((i) => i.id === item.id)) return items; + if (items.length === 0) return [item]; + const time = item.createdAt.getTime(); + const idx = items.findIndex((i) => i.createdAt.getTime() <= time); + if (idx === -1) return items; + return [...items.slice(0, idx), item, ...items.slice(idx)]; +} + +/** + * Fetches pages 1 to `pages` of a list again, and resolves to their items, + * each once, and whether more follow. An item added between two pages' + * fetches shifts the later page by one, which would list an item twice. A + * live list calls it after a reconnection, through `usePagedList`'s `resync` + * or, for the search's dumps, directly; it rejects when a page fails. + */ +export async function refetchPages( + pages: number, + fetchPage: (page: number) => Promise>, +): Promise> { + const results = await Promise.all( + Array.from({ length: pages }, (_, i) => fetchPage(i + 1)), + ); + const byId = new Map(); + for (const item of results.flatMap((r) => r.items)) { + if (!byId.has(item.id)) byId.set(item.id, item); + } + return { + items: [...byId.values()], + hasMore: results[results.length - 1].hasMore, + }; +}