From 786976312fc1f2edf5870433d7d872339bd30317 Mon Sep 17 00:00:00 2001 From: jcoffey <51408202+jcoffey-dev@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:15:21 -0700 Subject: [PATCH] Open conversations in one request, and start them early (#392) On a 250 ms link, opening a conversation took two round trips: Thread/get, then the bodies. It now takes one. A known thread sends Thread/get and the missing bodies in the same tick; an unknown one chains Email/get off Thread/get with a back-reference, and falls back to fetching in parts when the thread is longer than one Email/get may carry. Conversations also start loading before the click: when the pointer rests on a row, as soon as a press begins, and for the row below the open one. The open waits for that load and does not repeat it. Going back to one of the last twelve folders shows its previous list at once, less messages that have left it, while the query runs. --- web/src/store/__tests__/list-refresh.test.ts | 92 ++++++++++++- web/src/store/mail/index.ts | 131 ++++++++++++++++++- web/src/store/mail/types.ts | 2 + web/src/views/mail/MailView.tsx | 18 +++ web/src/views/mail/MessageList.tsx | 26 +++- 5 files changed, 261 insertions(+), 8 deletions(-) diff --git a/web/src/store/__tests__/list-refresh.test.ts b/web/src/store/__tests__/list-refresh.test.ts index ae06000..8685210 100644 --- a/web/src/store/__tests__/list-refresh.test.ts +++ b/web/src/store/__tests__/list-refresh.test.ts @@ -67,7 +67,7 @@ function server(count: number, threadSize = 1) { return { ok: true, status: 200, json: async () => ({ methodResponses: responses, sessionState: "1" }) } as Response; }); vi.stubGlobal("fetch", fetchMock); - return { calls, listed }; + return { calls, listed, requests: () => fetchMock.mock.calls.length }; } const getSizes = (calls: Call[]) => calls.filter(([n]) => n === "Email/get").map(([, a]) => (a.ids as string[]).length); @@ -79,6 +79,7 @@ beforeEach(() => { primaryAccounts: {}, state: "s1", } as unknown as JmapSession; + useMail.getState().setAccount("a1"); useMail.setState({ accountId: "a1", mailboxes: { [INBOX]: { id: INBOX, role: "inbox", name: "Inbox" } } as never, @@ -177,8 +178,11 @@ describe("merging a refresh", () => { }); describe("loadThread", () => { + const known = (t: string, emailIds: string[]) => useMail.setState({ threads: { [t]: { id: t, emailIds } } }); + it("fetches no bodies for messages already held in full", async () => { const { calls } = server(1, 3); + known("te0", ["e0", "e0m0", "e0m1"]); useMail.setState({ emails: { e0: { id: "e0" }, e0m0: { id: "e0m0" }, e0m1: { id: "e0m1" } } as never, fullIds: { e0: true, e0m0: true, e0m1: true }, @@ -191,13 +195,15 @@ describe("loadThread", () => { expect(useMail.getState().emails.e0).toBe(before); }); - it("fetches in full only the message it does not have", async () => { - const { calls } = server(1, 3); + it("fetches in full only the message it does not have, in the same request as the thread", async () => { + const { calls, requests } = server(1, 3); + known("te0", ["e0", "e0m0", "e0m1"]); useMail.setState({ emails: { e0: { id: "e0" }, e0m0: { id: "e0m0" }, e0m1: { id: "e0m1" } } as never, fullIds: { e0: true, e0m0: true }, }); await useMail.getState().loadThread("te0"); + expect(requests()).toBe(1); const gets = calls.filter(([n]) => n === "Email/get"); expect(gets).toHaveLength(1); expect(gets[0]![1].ids).toEqual(["e0m1"]); @@ -206,10 +212,90 @@ describe("loadThread", () => { expect(useMail.getState().loadingThreads).toEqual({}); }); + it("opens a thread it has never seen in one request", async () => { + const { calls, requests } = server(1, 3); + const got = await useMail.getState().loadThread("te0"); + expect(requests()).toBe(1); + expect(calls.map(([n]) => n)).toEqual(["Thread/get", "Email/get"]); + expect(calls[1]![1].fetchHTMLBodyValues).toBe(true); + expect(got.map((e) => e.id)).toEqual(["e0", "e0m0", "e0m1"]); + expect(useMail.getState().fullIds).toEqual({ e0: true, e0m0: true, e0m1: true }); + expect(useMail.getState().threads.te0?.emailIds).toEqual(["e0", "e0m0", "e0m1"]); + }); + it("splits a thread longer than one Email/get may carry", async () => { const { calls } = server(1, 1200); + known("te0", ["e0", ...Array.from({ length: 1199 }, (_, j) => `e0m${j}`)]); const got = await useMail.getState().loadThread("te0"); expect(got).toHaveLength(1200); expect(getSizes(calls).every((n) => n <= MAX)).toBe(true); }); + + it("falls back to parts when a thread it has never seen is too long for one request", async () => { + const { calls } = server(1, 1200); + const got = await useMail.getState().loadThread("te0"); + expect(got).toHaveLength(1200); + // The refused call, then the members in parts the server takes. + expect(getSizes(calls)).toEqual([1200, 500, 500, 200]); + }); +}); + +describe("prefetchThread", () => { + it("is the request a later open waits for", async () => { + const { requests } = server(1, 2); + useMail.getState().prefetchThread("te0"); + const got = await useMail.getState().loadThread("te0"); + expect(got.map((e) => e.id)).toEqual(["e0", "e0m0"]); + // The open waits for the prefetch and asks for nothing more. + expect(requests()).toBe(1); + }); + + it("does nothing for a conversation already held in full", async () => { + const { requests } = server(1, 1); + useMail.setState({ threads: { te0: { id: "te0", emailIds: ["e0"] } }, emails: { e0: { id: "e0" } } as never, fullIds: { e0: true } }); + useMail.getState().prefetchThread("te0"); + await Promise.resolve(); + expect(requests()).toBe(0); + }); + + it("asks once however often it is asked", async () => { + const { requests } = server(1, 1); + useMail.getState().prefetchThread("te0"); + useMail.getState().prefetchThread("te0"); + useMail.getState().prefetchThread("te0"); + await vi.waitFor(() => expect(useMail.getState().fullIds.e0).toBe(true)); + expect(requests()).toBe(1); + }); +}); + +describe("going back to a folder", () => { + const folder = (mailboxId: string) => ({ key: "", filter: { inMailbox: mailboxId }, sort: [], collapseThreads: false, mailboxId }); + + it("shows its last list while the query is on its way, less what has left it", async () => { + server(3); + await useMail.getState().query(folder(INBOX)); + expect(useMail.getState().list!.ids).toEqual(["e0", "e1", "e2"]); + await useMail.getState().query(folder("mbOther")); + // e1 is moved away meanwhile. + useMail.setState((s) => ({ emails: { ...s.emails, e1: { ...s.emails.e1!, mailboxIds: { mbOther: true } } } })); + const back = useMail.getState().query(folder(INBOX)); + const shown = useMail.getState().list!; + expect(shown.loading).toBe(true); + expect(shown.ids).toEqual(["e0", "e2"]); + expect(shown.total).toBe(2); + await back; + expect(useMail.getState().list!.loading).toBe(false); + }); + + it("starts empty for a folder not read before, and for a search", async () => { + server(3); + await useMail.getState().query(folder(INBOX)); + void useMail.getState().query(folder("mbNever")); + expect(useMail.getState().list!.ids).toEqual([]); + const search = { filter: { inMailbox: INBOX, text: "x" }, sort: [], collapseThreads: false, mailboxId: INBOX }; + await useMail.getState().query(search as never); + await useMail.getState().query(folder(INBOX)); + void useMail.getState().query(search as never); + expect(useMail.getState().list!.ids).toEqual([]); + }); }); diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index 0d7388d..be3a66e 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -19,6 +19,7 @@ import type { VacationResponse, ChangesResponse, Invocation, + MethodError, } from "@/jmap/types"; import { toast } from "@/ui/toast"; import { settings, useSettings } from "../settings"; @@ -101,6 +102,7 @@ export const useMail = create((set, get) => ({ setAccount(accountId) { if (accountId === get().accountId) return; resetBodyOrder(); + snapshots.clear(); set({ accountId, mailboxes: {}, @@ -169,8 +171,10 @@ export const useMail = create((set, get) => ({ void get().refreshList(); return; } + if (cur && cur.key !== key) keepSnapshot(cur); + const shown = reuse ? { ids: cur.ids, total: cur.total } : snapshotFor(key, q.filter, get().emails); set({ - list: { ...q, key, ids: reuse ? cur.ids : [], total: reuse ? cur.total : 0, queryState: null, loading: true, loadingMore: false, error: null, exhausted: false }, + list: { ...q, key, ids: shown.ids, total: shown.total, queryState: null, loading: true, loadingMore: false, error: null, exhausted: false }, selected: {}, selectedAll: false, anchorId: null, @@ -290,8 +294,7 @@ export const useMail = create((set, get) => ({ * held in full needs nothing more. getEmails also splits the fetch to * `maxObjectsInGet`, which a long thread could exceed. */ - const res = await client.call>("Thread/get", { accountId, ids: [threadId] }); - const thread = res.list[0]; + const thread = await fetchThread(accountId, threadId, get); if (!thread) { set((s) => { const { [threadId]: _drop, ...rest } = s.loadingThreads; @@ -299,7 +302,6 @@ export const useMail = create((set, get) => ({ }); return []; } - await get().getEmails(thread.emailIds, true); set((s) => { const { [threadId]: _drop, ...rest } = s.loadingThreads; return { threads: { ...s.threads, [threadId]: thread }, loadingThreads: rest }; @@ -314,6 +316,21 @@ export const useMail = create((set, get) => ({ } }, + prefetchThread(threadId) { + const { accountId, threads, fullIds } = get(); + if (!accountId || prefetched.has(threadId)) return; + const known = threads[threadId]; + if (known && known.emailIds.every((id) => fullIds[id])) return; + const run = fetchThread(accountId, threadId, get) + .then((thread) => { + if (thread) set((s) => ({ threads: { ...s.threads, [threadId]: thread } })); + return thread; + }) + .catch(() => null) + .finally(() => prefetched.delete(threadId)); + prefetched.set(threadId, run); + }, + threadEmails(threadId) { const { threads, emails } = get(); const t = threads[threadId]; @@ -1114,6 +1131,112 @@ export function resetBodyOrder(): void { bodyOrder.length = 0; } +/* + * Opening a conversation in one round trip. + * + * It used to take two: Thread/get, then the bodies once the ids came back -- + * half a second on a 250 ms link before anything showed. When the list has + * already fetched the thread (it has, in conversation view), the missing + * bodies are asked for in the same tick as the Thread/get, and the client + * sends both in one request. When it has not, the two are chained with a + * back-reference, which is also one request. Either way a member the list did + * not know about is fetched afterwards, which is rare. + * + * Loads of the same thread share one request: a conversation fetched ahead of + * the click (`prefetchThread`) is the one the click then waits for, and what + * it brought back is not asked for again. + */ +const prefetched = new Map>(); + +async function fetchThread(accountId: Id, threadId: Id, get: () => MailState): Promise { + const ahead = prefetched.get(threadId); + if (ahead) { + const thread = await ahead; + if (thread && thread.emailIds.every((id) => get().fullIds[id])) return thread; + } + const { threads, fullIds } = get(); + const known = threads[threadId]; + let thread: Thread | undefined; + if (known) { + const missing = known.emailIds.filter((id) => !fullIds[id]); + const [res] = await Promise.all([ + client.call>("Thread/get", { accountId, ids: [threadId] }), + missing.length ? get().getEmails(missing, true) : Promise.resolve([]), + ]); + thread = res.list[0]; + } else { + const res = await client.chain([ + ["Thread/get", { accountId, ids: [threadId] }, "t"], + [ + "Email/get", + { + accountId, + "#ids": { resultOf: "t", name: "Thread/get", path: "/list/*/emailIds" }, + properties: FULL_PROPS, + fetchHTMLBodyValues: true, + fetchTextBodyValues: true, + maxBodyValueBytes: 2 * 1024 * 1024, + bodyProperties: BODY_PROPS, + }, + "e", + ], + ], { allowErrors: true }); + const threadRes = res.get("t")?.[0]; + if (threadRes && "__error" in threadRes) throw new JmapMethodError("Thread/get", threadRes.__error as MethodError); + thread = (threadRes as unknown as GetResponse | undefined)?.list[0]; + // A thread longer than one Email/get may carry is refused whole; the members are fetched in parts below. + const got = (res.get("e")?.[0] as unknown as Partial> | undefined)?.list ?? []; + if (got.length) { + useMail.setState((s) => { + const emails = { ...s.emails }; + const full = { ...s.fullIds }; + for (const e of got) { + emails[e.id] = mergeEmail(emails[e.id], e); + full[e.id] = true; + } + return { emails, fullIds: full }; + }); + touchBodies(got.map((e) => e.id)); + useMail.setState((s) => releaseBodies(s)); + } + } + if (!thread) return null; + const late = thread.emailIds.filter((id) => !get().fullIds[id]); + if (late.length) await get().getEmails(late, true); + return thread; +} + +/* + * The last few folders' lists, shown again while their query is on its way. + * Going back to a folder read a moment ago otherwise blanks the list for a + * round trip. Only plain folder views are kept: a message that has since left + * the folder is dropped here, and anything else that changed is corrected by + * the query a round trip later. + */ +const SNAPSHOTS_KEPT = 12; +const SNAPSHOT_IDS = 200; +const snapshots = new Map(); + +function folderOf(filter: EmailFilter): Id | null { + const keys = Object.keys(filter); + return keys.length === 1 && "inMailbox" in filter && typeof filter.inMailbox === "string" ? filter.inMailbox : null; +} + +function keepSnapshot(list: NonNullable): void { + if (list.loading || list.error || !folderOf(list.filter)) return; + snapshots.delete(list.key); + snapshots.set(list.key, { ids: list.ids.slice(0, SNAPSHOT_IDS), total: list.total }); + while (snapshots.size > SNAPSHOTS_KEPT) snapshots.delete(snapshots.keys().next().value!); +} + +function snapshotFor(key: string, filter: EmailFilter, emails: Record): { ids: Id[]; total: number } { + const snap = snapshots.get(key); + const folder = folderOf(filter); + if (!snap || !folder) return { ids: [], total: 0 }; + const ids = snap.ids.filter((id) => emails[id]?.mailboxIds[folder]); + return { ids, total: snap.total - (snap.ids.length - ids.length) }; +} + let sortRefused = false; async function runQuery(accountId: Id, q: ListQuery, position: number, limit: number) { diff --git a/web/src/store/mail/types.ts b/web/src/store/mail/types.ts index 929d9b7..2bbca7a 100644 --- a/web/src/store/mail/types.ts +++ b/web/src/store/mail/types.ts @@ -71,6 +71,8 @@ export interface MailState { getEmails(ids: Id[], full?: boolean): Promise; loadThread(threadId: Id): Promise; + /** Start loading a conversation that is likely to be opened next; quiet, and shared with a later loadThread. */ + prefetchThread(threadId: Id): void; threadEmails(threadId: Id): Email[]; threadIdsIn(threadId: Id, mailboxId: Id | null): Id[]; diff --git a/web/src/views/mail/MailView.tsx b/web/src/views/mail/MailView.tsx index aacecfc..8fbb21c 100644 --- a/web/src/views/mail/MailView.tsx +++ b/web/src/views/mail/MailView.tsx @@ -183,6 +183,24 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string; return -1; }, [ids, focusId, openMessageId, threadId, rowThreadId]); + /* + * Reading down a folder usually means the next row is next. Once the open + * conversation has had its turn, the one below starts loading while the + * reader reads, so moving on waits on nothing. + */ + const nextRowId = threadId && currentRowIndex >= 0 ? ids[currentRowIndex + 1] : undefined; + const nextThreadId = nextRowId ? rowThreadId(nextRowId) : undefined; + useEffect(() => { + if (!nextThreadId || nextThreadId === threadId) return; + const start = () => useMail.getState().prefetchThread(nextThreadId); + if (typeof window.requestIdleCallback === "function") { + const handle = window.requestIdleCallback(start, { timeout: 2000 }); + return () => window.cancelIdleCallback(handle); + } + const handle = window.setTimeout(start, 500); + return () => window.clearTimeout(handle); + }, [nextThreadId, threadId]); + /** Email ids affected by an action on rows (selection or focused/open row). */ const targetIds = useCallback( async (rowIds?: Id[]): Promise => { diff --git a/web/src/views/mail/MessageList.tsx b/web/src/views/mail/MessageList.tsx index cd549c7..2b339d1 100644 --- a/web/src/views/mail/MessageList.tsx +++ b/web/src/views/mail/MessageList.tsx @@ -1,4 +1,4 @@ -import { Fragment, lazy, memo, Suspense, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type DragEvent, type MouseEvent, type ReactNode } from "react"; +import { Fragment, lazy, memo, Suspense, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type DragEvent, type MouseEvent, type PointerEvent, type ReactNode } from "react"; import { useVirtualizer } from "@tanstack/react-virtual"; import { useShallow } from "zustand/react/shallow"; import { Archive, ArrowLeft, CalendarDays, CalendarRange, CalendarPlus, CheckSquare, FolderInput, PanelRight, PanelBottom, PanelTop, Filter, Inbox, Mail, MailOpen, MailPlus, MoreVertical, Paperclip, RefreshCw, Reply, Search, Star, Tag, Trash2, AlertOctagon, Forward, Eraser, ShieldCheck, X } from "lucide-react"; @@ -718,6 +718,27 @@ function RowView({ email: e, threadEmails, top, height, selected, focused, open, }, }); + /* + * A conversation starts loading when the pointer settles on its row, or the + * moment a finger or button goes down, so that on a slow link the click + * finds it on its way. Drafts open in the composer instead. + */ + const hover = useRef(undefined); + useEffect(() => () => window.clearTimeout(hover.current), []); + const prefetch = () => { + if (!isDrafts) useMail.getState().prefetchThread(e.threadId); + }; + const onPointerEnter = (ev: PointerEvent) => { + if (ev.pointerType !== "mouse") return; + window.clearTimeout(hover.current); + hover.current = window.setTimeout(prefetch, 80); + }; + const onPointerLeave = () => window.clearTimeout(hover.current); + const onPointerDown = (ev: PointerEvent) => { + prefetch(); + gesture.onPointerDown?.(ev); + }; + const onDragStart = (ev: DragEvent) => { // Read when the drag starts, so the row need not re-render on every change of selection. const selectedIds = useMail.getState().selected; @@ -754,6 +775,9 @@ function RowView({ email: e, threadEmails, top, height, selected, focused, open, draggable={!touch} onDragStart={onDragStart} {...gesture} + onPointerDown={onPointerDown} + onPointerEnter={onPointerEnter} + onPointerLeave={onPointerLeave} role="row" aria-selected={selected} >