Merge pull request #381 from Coffey-Labs/perf/message-list-rows

Render only the message rows that changed

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);

web/src/store/mail/index.ts

@@ -255,7 +255,7 @@ export const useMail = create<MailState>((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<MailState>((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<void> {
   );
   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 };
   });
 }

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<SwipeIcon, ReactNode> = {
   move: <FolderInput size={22} />,
 };
 
+/**
+ * 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<A extends unknown[], R>(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<void>;
   trash: (rows?: Id[]) => Promise<void>;
@@ -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) => {
+  const onContext = useStableCallback((e: MouseEvent, rowId: Id) => {
     e.preventDefault();
     setCtxRow(rowId);
     setFocusId(rowId);
     ctxMenu.openAt(e.clientX, e.clientY);
-    },
-    [ctxMenu, setFocusId],
-  );
+  });
 
   /*
    * 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) => {
+  const onLongPress = useStableCallback((rowId: Id) => {
     haptic(15);
     setFocusId(rowId);
     lastClick.current = rowId;
     select([rowId], !useMail.getState().selected[rowId]);
-    },
-    [select, setFocusId],
-  );
+  });
 
   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 <div key={id} style={{ position: "absolute", top: vi.start, height: vi.size }} />;
-                const thread = list?.collapseThreads ? threads[e.threadId] : undefined;
                 const strip = swiping?.id === id ? swiping : null;
                 return (
                   <Fragment key={id}>
@@ -486,8 +505,8 @@ export function MessageList({ title, list, openThreadId, openMessageId, focusId,
                       </div>
                     )}
                     <Row
-                      email={e}
-                      threadEmails={thread ? thread.emailIds.map((x) => 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<Id, true>;
   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<void>;
 }
 
-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 <RowView {...rest} email={email} threadEmails={threadId ? threadEmails : undefined} />;
+});
+
+type RowViewProps = Omit<RowProps, "id" | "threadId"> & { 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 scope = useMemo(() => {
     const inScope = threadEmails ? threadEmails.filter((x) => (mailboxId ? x.mailboxIds[mailboxId] : true)) : [e];
-  const scope = inScope.length ? inScope : [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<Id>();
@@ -762,4 +810,4 @@ const Row = memo(function Row({ email: e, threadEmails, top, height, selected, f
       )}
     </div>
   );
-});
+}