From 4887ed457c729722b31bc5ddd1ffb87eee52eebe Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 02:28:55 +0100 Subject: [PATCH] fix: stop list toolbar timers from firing after the toolbar is gone --- .../email/__tests__/mv-list-toolbar.test.tsx | 23 ++++++++++++- components/email/email-list.tsx | 18 +++++----- components/email/mv-list-toolbar.tsx | 4 ++- .../use-timeout-while-mounted.test.tsx | 34 +++++++++++++++++++ hooks/use-timeout-while-mounted.ts | 29 ++++++++++++++++ 5 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 hooks/__tests__/use-timeout-while-mounted.test.tsx create mode 100644 hooks/use-timeout-while-mounted.ts diff --git a/components/email/__tests__/mv-list-toolbar.test.tsx b/components/email/__tests__/mv-list-toolbar.test.tsx index c86e1475b..082bb667b 100644 --- a/components/email/__tests__/mv-list-toolbar.test.tsx +++ b/components/email/__tests__/mv-list-toolbar.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { act, render, screen, fireEvent, waitFor } from '@testing-library/react'; import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { Email, Mailbox } from '@/lib/jmap/types'; import { KEYWORD_PREFIX } from '@/lib/thread-utils'; @@ -338,4 +338,25 @@ describe('MvListToolbar removal confirmations', () => { await waitFor(() => expect(batchArchive).toHaveBeenCalledTimes(1)); expect(screen.queryByRole('alertdialog')).toBeNull(); }); + + it('leaves no timer running once it is gone', async () => { + // The buttons stay disabled for a moment after an action. A bare timer + // for that outlived the toolbar and reached React after the test + // environment was torn down ("window is not defined", failing CI). + vi.useFakeTimers(); + try { + useSettingsStore.setState({ confirmArchive: false } as never); + const { unmount } = render( {}} />); + await act(async () => { + fireEvent.click(screen.getByLabelText('archive')); + }); + expect(batchArchive).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBeGreaterThan(0); + + unmount(); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index aaab68082..cae4a73b9 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -48,6 +48,7 @@ import { PluginSlot } from "@/components/plugins/plugin-slot"; import { PluginListFilters } from "./plugin-list-filters"; import { PluginActions, useFolderAccountKind } from "@/components/plugins/plugin-actions"; import { useUnreadFirstSection } from "@/hooks/use-unread-first-section"; +import { useTimeoutWhileMounted } from "@/hooks/use-timeout-while-mounted"; import { unreadSectionRowCount } from "@/lib/unread-first"; import { MvListSectionHeader } from "./mv-list-section-header"; import { type OpenableEmail, PluginSearchResults } from "./plugin-search-results"; @@ -382,6 +383,7 @@ export function EmailList({ const [sourceEmail, setSourceEmail] = useState(null); const [isProcessing, setIsProcessing] = useState(false); + const later = useTimeoutWhileMounted(); const parentRef = useRef(null); const batchToolbarRef = useRef(null); // The batch toolbar sits above the scroll container, so as it animates @@ -557,7 +559,7 @@ export function EmailList({ try { await removeFromFolder(resolveSelectedEmails(useEmailStore.getState())); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; const listSlotProps = useMemo( @@ -584,7 +586,7 @@ export function EmailList({ try { await markSelectionRead(read); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -631,7 +633,7 @@ export function EmailList({ } catch { toast.error(tSpam('error_not_spam')); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -647,7 +649,7 @@ export function EmailList({ } catch { toast.error(tSpam('error')); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -667,7 +669,7 @@ export function EmailList({ undo: undoMoveAction(client, restore, undo), }); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -686,7 +688,7 @@ export function EmailList({ undo: undoMoveAction(client, restore, undo, receipts), }); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -724,7 +726,7 @@ export function EmailList({ undo: undoMoveAction(client, restore, undo), }); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; @@ -746,7 +748,7 @@ export function EmailList({ try { await emptyMailbox(client, currentMailbox.id); } finally { - setTimeout(() => setIsProcessing(false), 500); + later(() => setIsProcessing(false), 500); } }; diff --git a/components/email/mv-list-toolbar.tsx b/components/email/mv-list-toolbar.tsx index 1b1cebb10..66be1f128 100644 --- a/components/email/mv-list-toolbar.tsx +++ b/components/email/mv-list-toolbar.tsx @@ -39,6 +39,7 @@ import { getEmailTagIds } from "@/lib/thread-utils"; import { cn, type MailboxNode } from "@/lib/utils"; import { PluginActions } from "@/components/plugins/plugin-actions"; import { useUnreadFirstSection } from "@/hooks/use-unread-first-section"; +import { useTimeoutWhileMounted } from "@/hooks/use-timeout-while-mounted"; import { commonTagScope } from "@/hooks/use-account-tags"; interface MvListToolbarProps { @@ -156,6 +157,7 @@ export function MvListToolbar({ }))); const [isProcessing, setIsProcessing] = useState(false); + const later = useTimeoutWhileMounted(); // The ⋮, Move to and Tag menus are the ContextMenu, as every menu (R6). const overflowMenu = useContextMenu(); const menuOpen = overflowMenu.contextMenu.isOpen; @@ -217,7 +219,7 @@ export function MvListToolbar({ } } } finally { - setTimeout(() => setIsProcessing(false), 400); + later(() => setIsProcessing(false), 400); } }; diff --git a/hooks/__tests__/use-timeout-while-mounted.test.tsx b/hooks/__tests__/use-timeout-while-mounted.test.tsx new file mode 100644 index 000000000..7734d732c --- /dev/null +++ b/hooks/__tests__/use-timeout-while-mounted.test.tsx @@ -0,0 +1,34 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useTimeoutWhileMounted } from '@/hooks/use-timeout-while-mounted'; + +describe('useTimeoutWhileMounted', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('runs the callback after the delay while the component is mounted', () => { + vi.useFakeTimers(); + const callback = vi.fn(); + const { result } = renderHook(() => useTimeoutWhileMounted()); + + result.current(callback, 500); + vi.advanceTimersByTime(499); + expect(callback).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('drops every pending callback when the component unmounts', () => { + vi.useFakeTimers(); + const callback = vi.fn(); + const { result, unmount } = renderHook(() => useTimeoutWhileMounted()); + + result.current(callback, 500); + result.current(callback, 800); + unmount(); + vi.advanceTimersByTime(1000); + expect(callback).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(0); + }); +}); diff --git a/hooks/use-timeout-while-mounted.ts b/hooks/use-timeout-while-mounted.ts new file mode 100644 index 000000000..1015d1b5c --- /dev/null +++ b/hooks/use-timeout-while-mounted.ts @@ -0,0 +1,29 @@ +import { useCallback, useEffect, useRef } from "react"; + +/** + * setTimeout whose pending timers are cleared when the component unmounts. + * + * A toolbar that kept its buttons disabled for a moment after an action + * called setState from a bare timer. When the component was gone by then, + * the call still reached React; in a test whose environment had already + * been torn down that threw "window is not defined" and failed the run. + */ +export function useTimeoutWhileMounted(): (callback: () => void, ms: number) => void { + const timers = useRef(new Set>()); + + useEffect(() => { + const pending = timers.current; + return () => { + pending.forEach(clearTimeout); + pending.clear(); + }; + }, []); + + return useCallback((callback: () => void, ms: number) => { + const id = setTimeout(() => { + timers.current.delete(id); + callback(); + }, ms); + timers.current.add(id); + }, []); +}