diff --git a/web/src/store/__tests__/list-refresh.test.ts b/web/src/store/__tests__/list-refresh.test.ts index dea9c19..ae06000 100644 --- a/web/src/store/__tests__/list-refresh.test.ts +++ b/web/src/store/__tests__/list-refresh.test.ts @@ -152,6 +152,30 @@ describe("refreshList", () => { }); }); +describe("merging a refresh", () => { + it("keeps the object for a message that did not change, so its row need not render", async () => { + const { listed } = server(3); + await useMail.getState().query({ key: "", filter: { inMailbox: INBOX }, sort: [], collapseThreads: false, mailboxId: INBOX }); + const before = { ...useMail.getState().emails }; + await useMail.getState().refreshList(); + const after = useMail.getState().emails; + for (const id of listed) expect(after[id]).toBe(before[id]); + }); + + it("replaces the object for a message that did change", async () => { + const { listed } = server(2); + await useMail.getState().query({ key: "", filter: { inMailbox: INBOX }, sort: [], collapseThreads: false, mailboxId: INBOX }); + // Held as starred; the server says it is not. + useMail.setState((s) => ({ emails: { ...s.emails, [listed[0]!]: { ...s.emails[listed[0]!]!, keywords: { $flagged: true } } } })); + const held = useMail.getState().emails; + await useMail.getState().refreshList(); + const after = useMail.getState().emails; + expect(after[listed[0]!]).not.toBe(held[listed[0]!]); + expect(after[listed[0]!]!.keywords).toEqual({}); + expect(after[listed[1]!]).toBe(held[listed[1]!]); + }); +}); + describe("loadThread", () => { it("fetches no bodies for messages already held in full", async () => { const { calls } = server(1, 3); diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index 4f2bd1e..9b2ee3c 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -255,7 +255,7 @@ export const useMail = create((set, get) => ({ for (const r of results) { state = r.state; for (const e of r.list) { - next[e.id] = { ...next[e.id], ...e }; + next[e.id] = mergeEmail(next[e.id], e); if (full) nextFull[e.id] = true; } } @@ -990,7 +990,7 @@ export const useMail = create((set, get) => ({ ); set((s) => { const next = { ...s.emails }; - for (const r of results) for (const e of r.list) next[e.id] = { ...next[e.id], ...e }; + for (const r of results) for (const e of r.list) next[e.id] = mergeEmail(next[e.id], e); return { emails: next }; }); } @@ -1038,6 +1038,26 @@ function sortIdentities(list: Identity[], accountId: Id): Identity[] { * * Keyed by nothing: a refusal is about the server, and there is only one. */ +/** + * Fold freshly fetched properties into the copy already held. + * + * Returns the held object itself when nothing in `next` differs from it. A + * refresh fetches every listed message again, and a new object for each one + * -- the same data, a new identity -- made every row of the list render + * again after any change at all. + */ +function mergeEmail(prev: Email | undefined, next: Email): Email { + if (!prev) return next; + for (const key of Object.keys(next) as (keyof Email)[]) { + const a = prev[key]; + const b = next[key]; + if (a === b) continue; + if (a && b && typeof a === "object" && JSON.stringify(a) === JSON.stringify(b)) continue; + return { ...prev, ...next }; + } + return prev; +} + let sortRefused = false; async function runQuery(accountId: Id, q: ListQuery, position: number, limit: number) { @@ -1096,7 +1116,7 @@ async function runQueryOnce(accountId: Id, q: ListQuery, position: number, reque const following = useMail.getState().emailState !== null; useMail.setState((s) => { const emails = { ...s.emails }; - for (const e of emailsRes.list) emails[e.id] = { ...emails[e.id], ...e }; + for (const e of emailsRes.list) emails[e.id] = mergeEmail(emails[e.id], e); const threads = { ...s.threads }; for (const t of threadsRes?.list ?? []) threads[t.id] = t; return { emails, threads, emailState: s.emailState ?? emailsRes.state }; @@ -1115,7 +1135,7 @@ async function refreshEmails(accountId: Id, ids: Id[]): Promise { ); useMail.setState((s) => { const emails = { ...s.emails }; - for (const r of results) for (const e of r.list) emails[e.id] = { ...emails[e.id], ...e }; + for (const r of results) for (const e of r.list) emails[e.id] = mergeEmail(emails[e.id], e); return { emails }; }); } diff --git a/web/src/views/mail/MessageList.tsx b/web/src/views/mail/MessageList.tsx index 0e4890e..9af67a9 100644 --- a/web/src/views/mail/MessageList.tsx +++ b/web/src/views/mail/MessageList.tsx @@ -1,5 +1,6 @@ -import { Fragment, memo, useCallback, useEffect, useMemo, useRef, useState, type DragEvent, type MouseEvent, type ReactNode } from "react"; +import { Fragment, memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type DragEvent, type MouseEvent, 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"; import { useLocation } from "wouter"; import { useMail, type ListState } from "@/store/mail"; @@ -39,6 +40,26 @@ const SWIPE_ICON: Record = { move: , }; +/** + * A callback whose identity never changes but which always runs the latest + * version of `fn`. + * + * The rows are memoized, and every handler they are given has to keep its + * identity for that to mean anything. The handlers here read the selection, + * the list and the menu, all of which change constantly -- so as ordinary + * `useCallback`s they were new on nearly every render, and every visible row + * rendered again with them. + */ +function useStableCallback(fn: (...args: A) => R): (...args: A) => R { + const ref = useRef(fn); + // Updated after render rather than during it, so a render React throws away + // never leaves its version behind. Handlers only run on events, which come later. + useLayoutEffect(() => { + ref.current = fn; + }); + return useCallback((...args: A) => ref.current(...args), []); +} + export interface ListActions { archive: (rows?: Id[]) => Promise; trash: (rows?: Id[]) => Promise; @@ -71,7 +92,6 @@ interface Props { export function MessageList({ title, list, openThreadId, openMessageId, focusId, setFocusId, onOpen, actions, mailboxId, isSearch }: Props) { const [, navigate] = useLocation(); const emails = useMail((s) => s.emails); - const threads = useMail((s) => s.threads); const selected = useMail((s) => s.selected); const select = useMail((s) => s.select); const selectAll = useMail((s) => s.selectAll); @@ -161,7 +181,7 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, onPull: useCallback((y: number, armed: boolean, live: boolean) => setPull({ y, armed, live }), []), }); - const onRowClick = useCallback( + const onRowClick = useStableCallback( (e: MouseEvent, rowId: Id) => { const action = rowClick({ rowId, ids, anchor: lastClick.current, selected, @@ -179,18 +199,14 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, // leaves the rows looking smeared blue over the selection they meant. else window.getSelection()?.removeAllRanges(); }, - [ids, select, selected, isMobile, onOpen], ); - const onContext = useCallback( - (e: MouseEvent, rowId: Id) => { - e.preventDefault(); - setCtxRow(rowId); - setFocusId(rowId); - ctxMenu.openAt(e.clientX, e.clientY); - }, - [ctxMenu, setFocusId], - ); + const onContext = useStableCallback((e: MouseEvent, rowId: Id) => { + e.preventDefault(); + setCtxRow(rowId); + setFocusId(rowId); + ctxMenu.openAt(e.clientX, e.clientY); + }); /* * Hold a row to select it, the way the mail app the phone came with does. @@ -201,15 +217,12 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, * row *is* how you select it. Once one row is selected, plain taps toggle * the rest (see `onRowClick`), so this only has to open the mode. */ - const onLongPress = useCallback( - (rowId: Id) => { - haptic(15); - setFocusId(rowId); - lastClick.current = rowId; - select([rowId], !useMail.getState().selected[rowId]); - }, - [select, setFocusId], - ); + const onLongPress = useStableCallback((rowId: Id) => { + haptic(15); + setFocusId(rowId); + lastClick.current = rowId; + select([rowId], !useMail.getState().selected[rowId]); + }); const onSwipeState = useCallback((rowId: Id, state: { dir: -1 | 1; armed: boolean; desc: SwipeDescriptor } | null) => { // A row clearing itself must not clear a gesture that has since moved on @@ -217,7 +230,7 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, setSwiping((cur) => (state ? { id: rowId, ...state } : cur?.id === rowId ? null : cur)); }, []); - const fireSwipe = useCallback( + const fireSwipe = useStableCallback( async (rowId: Id, d: SwipeDescriptor) => { switch (d.action) { case "archive": await actions.archive([rowId]); break; @@ -229,8 +242,15 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, case "move": actions.move([rowId]); break; } }, - [actions], ); + const onRowSelect = useStableCallback((rowId: Id, on: boolean) => { + select([rowId], on); + lastClick.current = rowId; + }); + const onRowStar = useStableCallback((rowId: Id, on: boolean) => void actions.star(on, [rowId])); + const onRowArchive = useStableCallback((rowId: Id) => void actions.archive([rowId])); + const onRowTrash = useStableCallback((rowId: Id) => void actions.trash([rowId])); + const onRowRead = useStableCallback((rowId: Id, read: boolean) => void actions.read(read, [rowId])); const ctxTargets = useMemo(() => (ctxRow ? (selected[ctxRow] ? Object.keys(selected) : [ctxRow]) : []), [ctxRow, selected]); @@ -463,7 +483,6 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, } const e = emails[id]; if (!e) return
; - const thread = list?.collapseThreads ? threads[e.threadId] : undefined; const strip = swiping?.id === id ? swiping : null; return ( @@ -486,8 +505,8 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId,
)} emails[x]).filter((x): x is Email => Boolean(x)) : undefined} + id={id} + threadId={list?.collapseThreads ? e.threadId : null} top={vi.start} height={vi.size} selected={Boolean(selected[id])} @@ -501,12 +520,11 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, isSent={mailbox?.role === "sent"} onClick={onRowClick} onContext={onContext} - onSelect={(rowId, on) => { select([rowId], on); lastClick.current = rowId; }} - onStar={(rowId, on) => void actions.star(on, [rowId])} - onArchive={(rowId) => void actions.archive([rowId])} - onTrash={(rowId) => void actions.trash([rowId])} - onRead={(rowId, read) => void actions.read(read, [rowId])} - selectedIds={selected} + onSelect={onRowSelect} + onStar={onRowStar} + onArchive={onRowArchive} + onTrash={onRowTrash} + onRead={onRowRead} touch={isTouch} role={mailbox?.role ?? null} swipeLeft={settings.swipeLeft} @@ -548,8 +566,9 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId, } interface RowProps { - email: Email; - threadEmails?: Email[]; + id: Id; + /** The conversation to summarize, in conversation view; null for a single message. */ + threadId: Id | null; top: number; height: number; selected: boolean; @@ -561,7 +580,6 @@ interface RowProps { isDrafts: boolean; isSent: boolean; mailboxId: Id | null; - selectedIds: Record; onClick: (e: MouseEvent, id: Id) => void; onContext: (e: MouseEvent, id: Id) => void; onSelect: (id: Id, on: boolean) => void; @@ -579,12 +597,40 @@ interface RowProps { onSwipeFire: (id: Id, desc: SwipeDescriptor) => Promise; } -const Row = memo(function Row({ email: e, threadEmails, top, height, selected, focused, open, twoLine, showAvatar, showPreview, isDrafts, isSent, mailboxId, selectedIds, onClick, onContext, onSelect, onStar, onArchive, onTrash, onRead, touch, role, swipeLeft, swipeRight, onLongPress, onSwipeState, onSwipeFire }: RowProps) { +const NO_EMAILS: Email[] = []; + +/* + * A row reads its own message and conversation from the store, rather than + * being handed them. The list re-renders on every store write -- each star, + * each push, each thread loaded -- and objects built for a row in that render + * were new every time, so no row was ever skipped. Selected this way, a row + * renders when something it shows has changed, and not otherwise. + */ +const Row = memo(function Row(props: RowProps) { + const email = useMail((s) => s.emails[props.id]); + const threadEmails = useMail( + useShallow((s) => { + if (!props.threadId) return NO_EMAILS; + const ids = s.threads[props.threadId]?.emailIds; + if (!ids) return NO_EMAILS; + return ids.map((x) => s.emails[x]).filter((x): x is Email => Boolean(x)); + }), + ); + if (!email) return null; + const { id: _id, threadId, ...rest } = props; + return ; +}); + +type RowViewProps = Omit & { email: Email; threadEmails?: Email[] }; + +function RowView({ email: e, threadEmails, top, height, selected, focused, open, twoLine, showAvatar, showPreview, isDrafts, isSent, mailboxId, onClick, onContext, onSelect, onStar, onArchive, onTrash, onRead, touch, role, swipeLeft, swipeRight, onLongPress, onSwipeState, onSwipeFire }: RowViewProps) { const labels = useSettings((s) => s.settings.labels); // Subscribed purely so the row re-renders when the date format changes. useSettings((s) => dateTimeKey(s.settings)); - const inScope = threadEmails ? threadEmails.filter((x) => (mailboxId ? x.mailboxIds[mailboxId] : true)) : [e]; - const scope = inScope.length ? inScope : [e]; + const scope = useMemo(() => { + const inScope = threadEmails ? threadEmails.filter((x) => (mailboxId ? x.mailboxIds[mailboxId] : true)) : [e]; + return inScope.length ? inScope : [e]; + }, [threadEmails, mailboxId, e]); const unread = scope.some((x) => !x.keywords.$seen); const starred = scope.some((x) => x.keywords.$flagged); const hasAtt = scope.some((x) => x.hasAttachment); @@ -671,6 +717,8 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f }); 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; const ids = selectedIds[e.id] ? Object.keys(selectedIds) : [e.id]; // include thread emails in scope const all = new Set(); @@ -762,4 +810,4 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f )} ); -}); +}