From 69e520b5aabd6845ef711a45e6c2e28511fc053c Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sat, 11 Apr 2026 12:30:05 +0200 Subject: [PATCH] fix: prevent cmd+n note creation crash --- src/hooks/useMenuEvents.test.ts | 51 +++++++++++++++- src/hooks/useMenuEvents.ts | 105 ++++++++++++++++++++------------ src/hooks/useNoteCreation.ts | 50 +++++++-------- 3 files changed, 141 insertions(+), 65 deletions(-) diff --git a/src/hooks/useMenuEvents.test.ts b/src/hooks/useMenuEvents.test.ts index 0cd8e2e0..51cd77e7 100644 --- a/src/hooks/useMenuEvents.test.ts +++ b/src/hooks/useMenuEvents.test.ts @@ -1,5 +1,22 @@ -import { describe, it, expect, vi } from 'vitest' -import { dispatchMenuEvent, type MenuEventHandlers } from './useMenuEvents' +import { renderHook } from '@testing-library/react' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { useMenuEvents, dispatchMenuEvent, type MenuEventHandlers } from './useMenuEvents' + +const isTauriMock = vi.fn(() => false) +const listenMock = vi.fn() +const invokeMock = vi.fn().mockResolvedValue(undefined) + +vi.mock('../mock-tauri', () => ({ + isTauri: () => isTauriMock(), +})) + +vi.mock('@tauri-apps/api/event', () => ({ + listen: (...args: unknown[]) => listenMock(...args), +})) + +vi.mock('@tauri-apps/api/core', () => ({ + invoke: (...args: unknown[]) => invokeMock(...args), +})) function makeHandlers(): MenuEventHandlers { return { @@ -43,6 +60,36 @@ function makeHandlers(): MenuEventHandlers { } } +describe('useMenuEvents', () => { + beforeEach(() => { + vi.clearAllMocks() + isTauriMock.mockReturnValue(false) + }) + + it('cleans up a native menu listener even if unmounted before listen resolves', async () => { + isTauriMock.mockReturnValue(true) + + let resolveListen: ((teardown: () => void) => void) | null = null + const teardown = vi.fn() + + listenMock.mockImplementationOnce(() => new Promise((resolve) => { + resolveListen = resolve + })) + + const { unmount } = renderHook(() => useMenuEvents(makeHandlers())) + await vi.dynamicImportSettled() + + expect(listenMock).toHaveBeenCalledTimes(1) + + unmount() + + resolveListen?.(teardown) + await vi.dynamicImportSettled() + + expect(teardown).toHaveBeenCalledTimes(1) + }) +}) + describe('dispatchMenuEvent', () => { // View mode events it('view-editor-only sets editor-only mode', () => { diff --git a/src/hooks/useMenuEvents.ts b/src/hooks/useMenuEvents.ts index 6cb19faa..519f8341 100644 --- a/src/hooks/useMenuEvents.ts +++ b/src/hooks/useMenuEvents.ts @@ -57,50 +57,54 @@ function syncNativeMenuState(state: MenuStatePayload): void { .catch(() => {}) } -/** Dispatch a Tauri menu event ID to the matching handler. Exported for testing. */ -export function dispatchMenuEvent(id: string, h: MenuEventHandlers): void { - if (!isAppCommandId(id)) return - dispatchAppCommand(id, h) -} - -/** Listen for native macOS menu events and dispatch them to the appropriate handlers. */ -export function useMenuEvents(handlers: MenuEventHandlers) { - const ref = useRef(handlers) - ref.current = handlers - const hasActiveNote = handlers.activeTabPath !== null - const hasModifiedFiles = handlers.modifiedCount != null ? handlers.modifiedCount > 0 : undefined - const hasConflicts = handlers.conflictCount != null ? handlers.conflictCount > 0 : undefined - const hasRestorableDeletedNote = handlers.hasRestorableDeletedNote - - // Subscribe once to Tauri menu events +function useNativeMenuEventListener(handlersRef: { current: MenuEventHandlers }) { useEffect(() => { if (!isTauri()) return - let cleanup: (() => void) | undefined - import('@tauri-apps/api/event').then(({ listen }) => { - const unlisten = listen('menu-event', (event) => { - dispatchMenuEvent(event.payload, ref.current) + let disposed = false + let unlisten: (() => void) | null = null + + import('@tauri-apps/api/event') + .then(async ({ listen }) => { + const teardown = await listen('menu-event', (event) => { + dispatchMenuEvent(event.payload, handlersRef.current) + }) + + if (disposed) { + teardown() + return + } + + unlisten = teardown + }) + .catch(() => { + /* not in Tauri */ }) - cleanup = () => { unlisten.then(fn => fn()) } - }).catch(() => { /* not in Tauri */ }) - return () => cleanup?.() - }, []) + return () => { + disposed = true + unlisten?.() + } + }, [handlersRef]) +} +function useWindowAppCommandListener(handlersRef: { current: MenuEventHandlers }) { useEffect(() => { const handleCommandEvent = createWindowCommandListener((detail) => { if (isAppCommandId(detail)) { - dispatchAppCommand(detail, ref.current) + dispatchAppCommand(detail, handlersRef.current) } }) window.addEventListener(APP_COMMAND_EVENT_NAME, handleCommandEvent) return () => window.removeEventListener(APP_COMMAND_EVENT_NAME, handleCommandEvent) - }, []) + }, [handlersRef]) +} +function useTestMenuCommandBridge(handlersRef: { current: MenuEventHandlers }) { useEffect(() => { const bridge = (id: string) => { - dispatchMenuEvent(id, ref.current) + dispatchMenuEvent(id, handlersRef.current) } window.__laputaTest = { @@ -113,15 +117,40 @@ export function useMenuEvents(handlers: MenuEventHandlers) { delete window.__laputaTest.dispatchBrowserMenuCommand } } - }, []) - - // Sync menu item enabled state when active note or git state changes - useEffect(() => { - syncNativeMenuState({ - hasActiveNote, - hasModifiedFiles, - hasConflicts, - hasRestorableDeletedNote, - }) - }, [hasActiveNote, hasModifiedFiles, hasConflicts, hasRestorableDeletedNote]) + }, [handlersRef]) +} + +function useNativeMenuStateSync(state: MenuStatePayload) { + useEffect(() => { + syncNativeMenuState(state) + }, [state]) +} + +/** Dispatch a Tauri menu event ID to the matching handler. Exported for testing. */ +export function dispatchMenuEvent(id: string, h: MenuEventHandlers): void { + if (!isAppCommandId(id)) return + dispatchAppCommand(id, h) +} + +/** Listen for native macOS menu events and dispatch them to the appropriate handlers. */ +export function useMenuEvents(handlers: MenuEventHandlers) { + const ref = useRef(handlers) + const hasActiveNote = handlers.activeTabPath !== null + const hasModifiedFiles = handlers.modifiedCount != null ? handlers.modifiedCount > 0 : undefined + const hasConflicts = handlers.conflictCount != null ? handlers.conflictCount > 0 : undefined + const hasRestorableDeletedNote = handlers.hasRestorableDeletedNote + + useEffect(() => { + ref.current = handlers + }, [handlers]) + + useNativeMenuEventListener(ref) + useWindowAppCommandListener(ref) + useTestMenuCommandBridge(ref) + useNativeMenuStateSync({ + hasActiveNote, + hasModifiedFiles, + hasConflicts, + hasRestorableDeletedNote, + }) } diff --git a/src/hooks/useNoteCreation.ts b/src/hooks/useNoteCreation.ts index c5554a7e..657df24d 100644 --- a/src/hooks/useNoteCreation.ts +++ b/src/hooks/useNoteCreation.ts @@ -310,25 +310,23 @@ export function useNoteCreation(config: NoteCreationConfig, tabDeps: CreationTab const queuedImmediateCreatesRef = useRef([]) const immediateCreateLockedRef = useRef(false) const immediateCreateTimerRef = useRef(null) - const latestImmediateCreateDepsRef = useRef({ - entries, - vaultPath: config.vaultPath, - pendingSlugs: pendingSlugsRef.current, - openTabWithContent, - addEntry, - trackUnsaved: config.trackUnsaved, - markContentPending: config.markContentPending, - }) + const latestImmediateCreateDepsRef = useRef(null) - latestImmediateCreateDepsRef.current = { - entries, - vaultPath: config.vaultPath, - pendingSlugs: pendingSlugsRef.current, - openTabWithContent, - addEntry, - trackUnsaved: config.trackUnsaved, - markContentPending: config.markContentPending, - } + const syncImmediateCreateDeps = useCallback(() => { + latestImmediateCreateDepsRef.current = { + entries, + vaultPath: config.vaultPath, + pendingSlugs: pendingSlugsRef.current, + openTabWithContent, + addEntry, + trackUnsaved: config.trackUnsaved, + markContentPending: config.markContentPending, + } + }, [entries, config.vaultPath, openTabWithContent, addEntry, config.trackUnsaved, config.markContentPending]) + + useEffect(() => { + syncImmediateCreateDeps() + }, [syncImmediateCreateDeps]) const persistNew: PersistFn = useCallback( (resolved) => createAndPersist(resolved, addEntry, openTabWithContent, { @@ -347,12 +345,13 @@ export function useNoteCreation(config: NoteCreationConfig, tabDeps: CreationTab }, [entries, persistNew, config.vaultPath]) const executeImmediateCreateRequest = useCallback((request: ImmediateCreateRequest) => { - createNoteImmediate(latestImmediateCreateDepsRef.current, request.type) + const deps = latestImmediateCreateDepsRef.current + if (!deps) return + createNoteImmediate(deps, request.type) trackEvent('note_created', { has_type: request.type ? 1 : 0, creation_path: request.type ? 'type_section' : 'cmd_n' }) }, []) - const continueImmediateCreateBurstRef = useRef<() => void>(() => {}) - continueImmediateCreateBurstRef.current = () => { + const continueImmediateCreateBurst = useCallback(function scheduleImmediateCreateBurst() { if (immediateCreateTimerRef.current !== null) return immediateCreateTimerRef.current = window.setTimeout(() => { @@ -364,11 +363,12 @@ export function useNoteCreation(config: NoteCreationConfig, tabDeps: CreationTab } executeImmediateCreateRequest(next) - continueImmediateCreateBurstRef.current() + scheduleImmediateCreateBurst() }, RAPID_CREATE_NOTE_SETTLE_MS) - } + }, [executeImmediateCreateRequest]) const handleCreateNoteImmediate = useCallback((type?: string) => { + syncImmediateCreateDeps() const request = { type } if (immediateCreateLockedRef.current) { queuedImmediateCreatesRef.current.push(request) @@ -377,8 +377,8 @@ export function useNoteCreation(config: NoteCreationConfig, tabDeps: CreationTab immediateCreateLockedRef.current = true executeImmediateCreateRequest(request) - continueImmediateCreateBurstRef.current() - }, [executeImmediateCreateRequest]) + continueImmediateCreateBurst() + }, [syncImmediateCreateDeps, executeImmediateCreateRequest, continueImmediateCreateBurst]) useEffect(() => () => { if (immediateCreateTimerRef.current !== null) {