From 47762bedf690beb7598aa440081250e7e0258c85 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:41:07 +0200 Subject: [PATCH] perf: re-render only the message rows that changed --- .../mail-list-plugin-surfaces.test.tsx | 4 + .../email/__tests__/thread-row-memo.test.ts | 96 +++++++++++++++++++ components/email/email-list.tsx | 94 ++++++++++++------ components/email/thread-list-item.tsx | 78 ++++++++++++++- hooks/__tests__/use-stable-handlers.test.ts | 52 ++++++++++ hooks/use-stable-handlers.ts | 39 ++++++++ 6 files changed, 332 insertions(+), 31 deletions(-) create mode 100644 components/email/__tests__/thread-row-memo.test.ts create mode 100644 hooks/__tests__/use-stable-handlers.test.ts create mode 100644 hooks/use-stable-handlers.ts diff --git a/components/email/__tests__/mail-list-plugin-surfaces.test.tsx b/components/email/__tests__/mail-list-plugin-surfaces.test.tsx index cba16b2d7..74e737138 100644 --- a/components/email/__tests__/mail-list-plugin-surfaces.test.tsx +++ b/components/email/__tests__/mail-list-plugin-surfaces.test.tsx @@ -219,6 +219,8 @@ describe('message-row-actions', () => { const onSelect = vi.fn(); const [thread] = groupEmailsByThread([mail('a')]); render( {}} onEmailSelect={onSelect} />); + // The hover actions mount once the row has been pointed at. + fireEvent.pointerEnter(screen.getByTestId('email-list-item')); const button = screen.getByRole('button', { name: 'Later' }); const star = screen.getByTestId('hover-action-star'); @@ -234,12 +236,14 @@ describe('message-row-actions', () => { useSettingsStore.setState({ hoverActions: [] } as never); act(() => setPluginActions('p', 'P', 'message-row-actions', [{ id: 'later', label: 'Later' }])); render(row(mail('a'))); + fireEvent.pointerEnter(screen.getByTestId('email-list-item')); expect(screen.getByRole('button', { name: 'Later' })).toBeInTheDocument(); }); it('honours the folder roles the plugin asked for', () => { act(() => setPluginActions('p', 'P', 'message-row-actions', [{ id: 'later', label: 'Later', mailboxRoles: ['sent'] }])); render(row(mail('a'))); + fireEvent.pointerEnter(screen.getByTestId('email-list-item')); expect(screen.queryByRole('button', { name: 'Later' })).toBeNull(); }); diff --git a/components/email/__tests__/thread-row-memo.test.ts b/components/email/__tests__/thread-row-memo.test.ts new file mode 100644 index 000000000..901aaf13b --- /dev/null +++ b/components/email/__tests__/thread-row-memo.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest'; +import type { Email, ThreadGroup } from '@/lib/jmap/types'; +import { areThreadRowPropsEqual } from '../thread-list-item'; + +type RowProps = Parameters[0]; + +const email = (id: string, keywords: Record = {}): Email => + ({ id, threadId: 't1', subject: `Subject ${id}`, keywords, receivedAt: '2026-09-21T10:00:00Z' }) as unknown as Email; + +const group = (emails: Email[], over: Partial = {}): ThreadGroup => ({ + threadId: 't1', + threadKey: 'acct:t1', + emails, + latestEmail: emails[0], + participantNames: ['Ada'], + hasUnread: false, + hasStarred: false, + hasPinned: false, + hasAttachment: false, + hasAnswered: false, + hasForwarded: false, + emailCount: emails.length, + ...over, +}); + +const onToggleExpand = () => {}; +const onEmailSelect = () => {}; +const props = (over: Partial): RowProps => ({ + thread: group([email('e1')]), + isExpanded: false, + onToggleExpand, + onEmailSelect, + ...over, +}); + +describe('areThreadRowPropsEqual', () => { + it('treats a regrouped thread holding the very same messages as unchanged', () => { + const e1 = email('e1'); + // groupEmailsByThread builds new ThreadGroup objects on every pass. + expect(areThreadRowPropsEqual(props({ thread: group([e1]) }), props({ thread: group([e1]) }))).toBe(true); + }); + + it('re-renders when a message in the thread was replaced (read, starred, tagged ...)', () => { + const before = email('e1'); + const after = email('e1', { $seen: true }); + expect(areThreadRowPropsEqual(props({ thread: group([before]) }), props({ thread: group([after]) }))).toBe(false); + }); + + it('re-renders when the late thread count arrives', () => { + const e1 = email('e1'); + expect(areThreadRowPropsEqual( + props({ thread: group([e1], { emailCount: 1 }) }), + props({ thread: group([e1], { emailCount: 3 }) }), + )).toBe(false); + }); + + it('re-renders when a thread flag or the participants change', () => { + const e1 = email('e1'); + const base = props({ thread: group([e1]) }); + expect(areThreadRowPropsEqual(base, props({ thread: group([e1], { hasUnread: true }) }))).toBe(false); + expect(areThreadRowPropsEqual(base, props({ thread: group([e1], { participantNames: ['Ada', 'Grace'] }) }))).toBe(false); + }); + + it('ignores the open message moving between two OTHER rows', () => { + const thread = group([email('e1')]); + expect(areThreadRowPropsEqual( + props({ thread, selectedEmailId: 'other-1' }), + props({ thread, selectedEmailId: 'other-2' }), + )).toBe(true); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, selectedEmailId: 'other-1' }))).toBe(true); + }); + + it('re-renders the row that gains or loses the open message', () => { + const thread = group([email('e1'), email('e2')]); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'other' }), props({ thread, selectedEmailId: 'e2' }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'e2' }), props({ thread, selectedEmailId: 'other' }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread, selectedEmailId: 'e1' }), props({ thread, selectedEmailId: 'e2' }))).toBe(false); + }); + + it('counts messages shown by the expanded thread as drawn by the row', () => { + const thread = group([email('e1')]); + const expandedEmails = [email('e1'), email('e9')]; + expect(areThreadRowPropsEqual( + props({ thread, expandedEmails, isExpanded: true, selectedEmailId: 'other' }), + props({ thread, expandedEmails, isExpanded: true, selectedEmailId: 'e9' }), + )).toBe(false); + }); + + it('re-renders on any other prop change, including a handler appearing', () => { + const thread = group([email('e1')]); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, isExpanded: true }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, isLoading: true }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, onDelete: () => {} }))).toBe(false); + expect(areThreadRowPropsEqual(props({ thread }), props({ thread, onEmailSelect: () => {} }))).toBe(false); + }); +}); diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 0ee95d697..55bde1a5e 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -1,7 +1,8 @@ "use client"; import { Email, ThreadGroup } from "@/lib/jmap/types"; -import { ThreadListItem } from "./thread-list-item"; +import { MemoizedThreadListItem } from "./thread-list-item"; +import { useStableHandlers } from "@/hooks/use-stable-handlers"; import type { Attachment } from "@/lib/jmap/types"; import type { LoadListAttachments } from "@/lib/list-attachments"; import { listRowShowsChips } from "./attachment-chips"; @@ -591,6 +592,45 @@ export function EmailList({ for (const [key, emails] of members) emitThreadExpand(key, emails, false); }, [collapseAllThreads, threadMembers]); + // Rows are memoised (MemoizedThreadListItem), which only pays off if what + // they are handed keeps its identity from one render to the next. + const rowHandlers = useStableHandlers({ + onToggleExpandKey: handleToggleThreadExpansion, + onCollapseAllThreads: collapseAllThreadsAndNotify, + onEmailSelect: (email: Email) => { + // Collapse expanded threads when selecting an email outside the expanded thread + const currentExpanded = useEmailStore.getState().expandedThreadIds; + if (currentExpanded.size > 0) { + // Check if the selected email belongs to any expanded thread via its threadId + if (!email.threadId || !currentExpanded.has(threadKeyFor(email))) { + collapseAllThreadsAndNotify(); + } + } + onEmailSelect?.(email); + }, + onEmailDoubleClick, + onContextMenu: openContextMenu, + onOpenConversation, + onToggleStar, + onMarkAsRead, + onDelete, + onArchive, + onSetTag, + onMarkAsSpam, + onUndoSpam, + onOpenAttachment, + }); + // One expand toggle per thread, kept for as long as the list lives. + const toggleExpandByThread = useRef(new Map void>()); + const toggleExpandFor = (threadKey: string) => { + let toggle = toggleExpandByThread.current.get(threadKey); + if (!toggle) { + toggle = () => rowHandlers.onToggleExpandKey(threadKey); + toggleExpandByThread.current.set(threadKey, toggle); + } + return toggle; + }; + // Range-based load more: trigger when last visible item is near the end. // Debounce to prevent rapid cascade when thread grouping reduces item // count below the viewport size (e.g. 2400 emails → fewer thread groups). @@ -620,9 +660,15 @@ export function EmailList({ thread.latestEmail.id === selectedEmailId || thread.emails.some(e => e.id === selectedEmailId) ); - if (index >= 0) { - virtualizer.scrollToIndex(index, { align: 'auto' }); - } + if (index < 0) return; + // A row clicked in the list is already on screen; scrollToIndex would + // still measure and schedule a scroll. Judge from the virtualizer's own + // numbers, which cost no layout read. + const item = virtualizer.measurementsCache[index]; + const top = virtualizer.scrollOffset; + const height = virtualizer.scrollRect?.height; + if (item && top !== null && height !== undefined && item.start >= top && item.end <= top + height) return; + virtualizer.scrollToIndex(index, { align: 'auto' }); // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedEmailId]); @@ -844,36 +890,26 @@ export function EmailList({ width: '100%', }} > - handleToggleThreadExpansion(thread.threadKey)} - onCollapseAllThreads={collapseAllThreadsAndNotify} - onEmailSelect={(email) => { - // Collapse expanded threads when selecting an email outside the expanded thread - const currentExpanded = useEmailStore.getState().expandedThreadIds; - if (currentExpanded.size > 0) { - // Check if the selected email belongs to any expanded thread via its threadId - if (!email.threadId || !currentExpanded.has(threadKeyFor(email))) { - collapseAllThreadsAndNotify(); - } - } - onEmailSelect?.(email); - }} - onEmailDoubleClick={onEmailDoubleClick ? (email) => onEmailDoubleClick(email) : undefined} - onContextMenu={openContextMenu} - onOpenConversation={onOpenConversation} - onToggleStar={onToggleStar ? (email) => onToggleStar(email) : undefined} - onMarkAsRead={onMarkAsRead ? (email, read) => onMarkAsRead(email, read) : undefined} - onDelete={onDelete ? (email) => onDelete(email) : undefined} - onArchive={onArchive ? (email) => onArchive(email) : undefined} - onSetTag={onSetTag} - onMarkAsSpam={onMarkAsSpam ? (email) => onMarkAsSpam(email) : undefined} - onUndoSpam={onUndoSpam ? (email) => onUndoSpam(email) : undefined} - onOpenAttachment={onOpenAttachment} + onToggleExpand={toggleExpandFor(thread.threadKey)} + onCollapseAllThreads={rowHandlers.onCollapseAllThreads} + onEmailSelect={rowHandlers.onEmailSelect} + onEmailDoubleClick={rowHandlers.onEmailDoubleClick} + onContextMenu={rowHandlers.onContextMenu} + onOpenConversation={rowHandlers.onOpenConversation} + onToggleStar={rowHandlers.onToggleStar} + onMarkAsRead={rowHandlers.onMarkAsRead} + onDelete={rowHandlers.onDelete} + onArchive={rowHandlers.onArchive} + onSetTag={rowHandlers.onSetTag} + onMarkAsSpam={rowHandlers.onMarkAsSpam} + onUndoSpam={rowHandlers.onUndoSpam} + onOpenAttachment={rowHandlers.onOpenAttachment} loadAttachments={loadAttachments} /> diff --git a/components/email/thread-list-item.tsx b/components/email/thread-list-item.tsx index c9bb86224..887b51a67 100644 --- a/components/email/thread-list-item.tsx +++ b/components/email/thread-list-item.tsx @@ -43,6 +43,19 @@ import { useTranslations } from "next-intl"; * it once the row grows a chip line. Extra-compact rows have no avatar and * anchor it on their single line instead (top padding + half a line). */ +/** + * Whether a row has been pointed at yet. The hover actions stay display:none + * until the pointer is over the row, so mounting them for every row up front + * only added a handful of icon buttons per row to every list render. They + * mount on the first pointer-enter and stay from then on, so the hover + * behaviour itself is unchanged. + */ +function useArmOnPointerEnter(): [boolean, () => void] { + const [armed, setArmed] = React.useState(false); + const arm = React.useCallback(() => setArmed(true), []); + return [armed, arm]; +} + function UnreadDot({ besideAvatar }: { besideAvatar: boolean }) { const t = useTranslations('email_viewer'); return ( @@ -372,6 +385,7 @@ const SingleEmailItem = React.forwardRef( sourceMailboxId: selectedMailbox, }); + const [hoverActionsArmed, armHoverActions] = useArmOnPointerEnter(); const { onTouchStart, onTouchEnd, onTouchMove, onTouchCancel, isPressed } = useLongPress( useCallback((pos) => { onContextMenu?.( @@ -462,6 +476,7 @@ const SingleEmailItem = React.forwardRef( {...dragHandlers} {...touchHandlers} data-testid="email-list-item" + onPointerEnter={armHoverActions} data-email-id={email.id} data-subject={email.subject || ''} data-unread={isUnread ? 'true' : 'false'} @@ -780,7 +795,7 @@ const SingleEmailItem = React.forwardRef( {/* Hover Quick Actions */} - {!email.isScheduled && ( + {hoverActionsArmed && !email.isScheduled && ( { onContextMenu?.( @@ -1028,6 +1044,7 @@ export const ThreadListItem = React.forwardRef {/* Hover Quick Actions for thread header */} - {!latestEmail.isScheduled && ( + {hoverActionsArmed && !latestEmail.isScheduled && ( email === b.emails[i]) + && a.participantNames.length === b.participantNames.length + && a.participantNames.every((name, i) => name === b.participantNames[i]); +} + +/** The open message, if this row draws it; otherwise none of the row's business. */ +function openMessageWithin(props: ThreadListItemProps): string | undefined { + const id = props.selectedEmailId; + if (!id) return undefined; + const drawn = props.thread.emails.some((email) => email.id === id) + || (props.expandedEmails?.some((email) => email.id === id) ?? false); + return drawn ? id : undefined; +} + +/** Props equality for MemoizedThreadListItem. */ +export function areThreadRowPropsEqual(prev: ThreadListItemProps, next: ThreadListItemProps): boolean { + const keys = new Set([...Object.keys(prev), ...Object.keys(next)]) as Set; + for (const key of keys) { + if (key === 'thread') { + if (!isSameThread(prev.thread, next.thread)) return false; + } else if (key === 'selectedEmailId') { + if (openMessageWithin(prev) !== openMessageWithin(next)) return false; + } else if (!Object.is(prev[key], next[key])) { + return false; + } + } + return true; +} + +/** + * ThreadListItem for the message list. Opening a message, switching folder or + * a late thread count re-rendered every row on screen; this one re-renders + * only when something it draws has changed. Handlers are compared by + * identity, so the list passes stable ones (see useStableHandlers). + */ +export const MemoizedThreadListItem = React.memo(ThreadListItem, areThreadRowPropsEqual); diff --git a/hooks/__tests__/use-stable-handlers.test.ts b/hooks/__tests__/use-stable-handlers.test.ts new file mode 100644 index 000000000..871729e2c --- /dev/null +++ b/hooks/__tests__/use-stable-handlers.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useStableHandlers } from '../use-stable-handlers'; + +describe('useStableHandlers', () => { + it('keeps each handler identity across renders while calling the latest closure', () => { + const first = vi.fn(); + const second = vi.fn(); + const { result, rerender } = renderHook(({ onPick }) => useStableHandlers({ onPick }), { + initialProps: { onPick: first as (id: string) => void }, + }); + + const stable = result.current; + rerender({ onPick: second }); + + expect(result.current).toBe(stable); + expect(result.current.onPick).toBe(stable.onPick); + + // A row that skipped the re-render still holds `stable.onPick`. + stable.onPick('e1'); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledWith('e1'); + }); + + it('passes arguments through and hands back the return value', () => { + const { result } = renderHook(() => useStableHandlers({ add: (a: number, b: number) => a + b })); + expect(result.current.add(2, 3)).toBe(5); + }); + + it('leaves an absent handler absent, and follows it when it appears or goes away', () => { + const onDelete = vi.fn(); + const { result, rerender } = renderHook( + ({ onDelete: handler }: { onDelete?: () => void }) => useStableHandlers({ onOpen: () => {}, onDelete: handler }), + { initialProps: {} as { onDelete?: () => void } }, + ); + + // Rows show a delete button only when there is something to call. + expect(result.current.onDelete).toBeUndefined(); + const withoutDelete = result.current; + + rerender({ onDelete }); + expect(result.current).not.toBe(withoutDelete); + expect(typeof result.current.onDelete).toBe('function'); + // Handlers that were there all along keep working through the old object too. + expect(() => withoutDelete.onOpen()).not.toThrow(); + result.current.onDelete?.(); + expect(onDelete).toHaveBeenCalledTimes(1); + + rerender({}); + expect(result.current.onDelete).toBeUndefined(); + }); +}); diff --git a/hooks/use-stable-handlers.ts b/hooks/use-stable-handlers.ts new file mode 100644 index 000000000..a93815e9a --- /dev/null +++ b/hooks/use-stable-handlers.ts @@ -0,0 +1,39 @@ +import { useLayoutEffect, useMemo, useRef } from "react"; + +type Handler = (...args: never[]) => unknown; +type Handlers = Record; + +/** + * Identity-stable stand-ins for a set of event handlers. + * + * A list that hands each row inline closures gives every row new props on + * every render, so memoising the rows achieves nothing. The handlers returned + * here keep their identity for the life of the component and forward to + * whatever was passed on the latest render, so a memoised row can be skipped + * without ever calling a stale closure. + * + * Only for handlers invoked after render (clicks, drags, keyboard): the latest + * set is published in a layout effect, so calling one during render would + * still see the previous render's closure. A handler that is `undefined` stays + * `undefined`, because rows use a handler's presence to decide what to show; + * the returned object only changes identity when that presence changes. + */ +export function useStableHandlers(handlers: T): T { + const latest = useRef(handlers); + useLayoutEffect(() => { + latest.current = handlers; + }); + + const present = Object.keys(handlers) + .filter((key) => handlers[key] !== undefined) + .sort() + .join(","); + + return useMemo(() => { + const stable: Handlers = {}; + for (const key of present ? present.split(",") : []) { + stable[key] = ((...args: never[]) => latest.current[key]?.(...args)) as Handler; + } + return stable as T; + }, [present]); +}