diff --git a/src/hooks/useNoteActions.hook.test.ts b/src/hooks/useNoteActions.hook.test.ts index 065aff2e..c2fe77ec 100644 --- a/src/hooks/useNoteActions.hook.test.ts +++ b/src/hooks/useNoteActions.hook.test.ts @@ -220,7 +220,11 @@ describe('useNoteActions hook', () => { await flushAsyncWork() }) await act(async () => { - vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS * 2) + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) + await flushAsyncWork() + }) + await act(async () => { + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) await flushAsyncWork() }) @@ -471,7 +475,11 @@ describe('useNoteActions hook', () => { await flushAsyncWork() }) await act(async () => { - vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS * 2) + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) + await flushAsyncWork() + }) + await act(async () => { + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) await flushAsyncWork() }) diff --git a/src/hooks/useNoteCreation.test.ts b/src/hooks/useNoteCreation.test.ts index c5316004..c8de546d 100644 --- a/src/hooks/useNoteCreation.test.ts +++ b/src/hooks/useNoteCreation.test.ts @@ -261,7 +261,11 @@ describe('useNoteCreation hook', () => { await flushImmediateCreate() }) await act(async () => { - vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS * 2) + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) + await flushImmediateCreate() + }) + await act(async () => { + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) await flushImmediateCreate() }) const filenames = addEntry.mock.calls.map(([e]: [VaultEntry]) => e.filename) @@ -326,6 +330,49 @@ describe('useNoteCreation hook', () => { vi.restoreAllMocks() }) + it('waits for slow immediate note persistence before starting the queued create', async () => { + vi.useFakeTimers() + vi.mocked(isTauri).mockReturnValue(true) + vi.spyOn(Date, 'now').mockReturnValue(1700000000000) + let resolveFirstWrite: () => void + const firstWrite = new Promise((resolve) => { + resolveFirstWrite = resolve + }) + vi.mocked(invoke) + .mockImplementationOnce(() => firstWrite) + .mockResolvedValue(undefined) + const createCalls = () => vi.mocked(invoke).mock.calls.filter(([command]) => command === 'create_note_content') + const { result } = renderHook(() => useNoteCreation(makeConfig(), tabDeps)) + + await act(async () => { + result.current.handleCreateNoteImmediate() + result.current.handleCreateNoteImmediate() + await flushImmediateCreate() + }) + + expect(createCalls()).toHaveLength(1) + + await act(async () => { + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS * 3) + await flushImmediateCreate() + }) + + expect(createCalls()).toHaveLength(1) + + await act(async () => { + resolveFirstWrite() + await flushImmediateCreate() + }) + await act(async () => { + vi.advanceTimersByTime(RAPID_CREATE_NOTE_SETTLE_MS) + await flushImmediateCreate() + }) + + expect(createCalls()).toHaveLength(2) + expect(addEntry).toHaveBeenCalledTimes(2) + vi.restoreAllMocks() + }) + it('handleCreateNoteImmediate accepts custom type', async () => { const { result } = renderHook(() => useNoteCreation(makeConfig(), tabDeps)) await act(async () => { diff --git a/src/hooks/useNoteCreation.ts b/src/hooks/useNoteCreation.ts index 21200831..532b8ee6 100644 --- a/src/hooks/useNoteCreation.ts +++ b/src/hooks/useNoteCreation.ts @@ -536,18 +536,26 @@ function useImmediateCreateQueue(config: ImmediateCreateQueueConfig): (type?: st const queuedImmediateCreatesRef = useRef([]) const immediateCreateLockedRef = useRef(false) const immediateCreateTimerRef = useRef(null) + const queueMountedRef = useRef(true) const { latestDepsRef, syncDeps } = useLatestImmediateCreateDeps(config, pendingSlugsRef) - const executeRequest = useCallback((request: ImmediateCreateRequest) => { + const executeRequest = useCallback(async (request: ImmediateCreateRequest): Promise => { const deps = latestDepsRef.current if (!deps) return - void createNoteImmediate(deps, request.type).then((didCreate) => trackImmediateCreate(request, didCreate)) + + try { + const didCreate = await createNoteImmediate(deps, request.type) + trackImmediateCreate(request, didCreate) + } catch (error) { + console.warn('Failed to create immediate note:', error) + } }, [latestDepsRef]) const scheduleQueuedBurst = useCallback(function scheduleQueuedBurst() { + if (!queueMountedRef.current) return if (immediateCreateTimerRef.current !== null) return - immediateCreateTimerRef.current = window.setTimeout(() => { + immediateCreateTimerRef.current = window.setTimeout(async () => { immediateCreateTimerRef.current = null const next = queuedImmediateCreatesRef.current.shift() if (!next) { @@ -555,14 +563,18 @@ function useImmediateCreateQueue(config: ImmediateCreateQueueConfig): (type?: st return } - executeRequest(next) + await executeRequest(next) scheduleQueuedBurst() }, RAPID_CREATE_NOTE_SETTLE_MS) }, [executeRequest]) - useEffect(() => () => { - if (immediateCreateTimerRef.current !== null) { - window.clearTimeout(immediateCreateTimerRef.current) + useEffect(() => { + queueMountedRef.current = true + return () => { + queueMountedRef.current = false + if (immediateCreateTimerRef.current !== null) { + window.clearTimeout(immediateCreateTimerRef.current) + } } }, []) @@ -575,8 +587,7 @@ function useImmediateCreateQueue(config: ImmediateCreateQueueConfig): (type?: st } immediateCreateLockedRef.current = true - executeRequest(request) - scheduleQueuedBurst() + void executeRequest(request).then(scheduleQueuedBurst) }, [syncDeps, executeRequest, scheduleQueuedBurst]) } diff --git a/tests/smoke/create-note-backing-file.spec.ts b/tests/smoke/create-note-backing-file.spec.ts index 915460bd..2ebe387e 100644 --- a/tests/smoke/create-note-backing-file.spec.ts +++ b/tests/smoke/create-note-backing-file.spec.ts @@ -5,7 +5,10 @@ import { triggerMenuCommand } from './testBridge' interface CreateNoteProbe { createCalls: string[] + startedCreateCalls: string[] getBeforeCreate: string[] + activeCreateCount: number + maxConcurrentCreates: number } interface ProbeWindow { @@ -14,6 +17,7 @@ interface ProbeWindow { } let tempVaultDir: string +const CREATE_NOTE_WRITE_DELAY_MS = 350 async function pinFixtureHandlers(page: Page): Promise { await page.evaluate(() => { @@ -32,22 +36,34 @@ async function pinFixtureHandlers(page: Page): Promise { } async function recordCreateNoteCalls(page: Page): Promise { - await page.evaluate(() => { + await page.evaluate((writeDelayMs) => { const probeWindow = window as typeof window & ProbeWindow const handlers = probeWindow.__mockHandlers as Record unknown> const originalCreate = handlers.create_note_content.bind(handlers) - const probe: CreateNoteProbe = { createCalls: [], getBeforeCreate: [] } + const probe: CreateNoteProbe = { + activeCreateCount: 0, + createCalls: [], + getBeforeCreate: [], + maxConcurrentCreates: 0, + startedCreateCalls: [], + } probeWindow.__createNoteBackingFileProbe = probe handlers.create_note_content = async (args?: unknown) => { const notePath = String((args as { path?: unknown } | undefined)?.path ?? '') - await new Promise((resolve) => setTimeout(resolve, 100)) - const result = await originalCreate(args) - probe.createCalls.push(notePath) - return result + probe.startedCreateCalls.push(notePath) + probe.activeCreateCount += 1 + probe.maxConcurrentCreates = Math.max(probe.maxConcurrentCreates, probe.activeCreateCount) + try { + await new Promise((resolve) => setTimeout(resolve, writeDelayMs)) + return await originalCreate(args) + } finally { + probe.activeCreateCount -= 1 + probe.createCalls.push(notePath) + } } - }) + }, CREATE_NOTE_WRITE_DELAY_MS) } async function rejectReadsBeforeCreate(page: Page): Promise { @@ -77,10 +93,36 @@ async function installCreateNoteBackingFileProbe(page: Page): Promise { async function readProbe(page: Page): Promise { return page.evaluate(() => { const probeWindow = window as typeof window & ProbeWindow - return probeWindow.__createNoteBackingFileProbe ?? { createCalls: [], getBeforeCreate: [] } + return probeWindow.__createNoteBackingFileProbe ?? { + activeCreateCount: 0, + createCalls: [], + getBeforeCreate: [], + maxConcurrentCreates: 0, + startedCreateCalls: [], + } }) } +async function dispatchMenuCommandBurst(page: Page, commandId: string, count: number): Promise { + await page.waitForFunction( + () => typeof window.__laputaTest?.dispatchBrowserMenuCommand === 'function', + undefined, + { timeout: 5_000 }, + ) + await page.evaluate(({ commandId: id, count: commandCount }) => { + const testWindow = window as typeof window & { + __laputaTest?: { dispatchBrowserMenuCommand?: (commandId: string) => void } + } + const dispatchBrowserMenuCommand = testWindow.__laputaTest?.dispatchBrowserMenuCommand + if (typeof dispatchBrowserMenuCommand !== 'function') { + throw new Error('Tolaria test bridge is missing dispatchBrowserMenuCommand') + } + for (let index = 0; index < commandCount; index += 1) { + dispatchBrowserMenuCommand(id) + } + }, { commandId, count }) +} + test.beforeEach(async ({ page }, testInfo) => { testInfo.setTimeout(60_000) tempVaultDir = createFixtureVaultCopy() @@ -112,3 +154,29 @@ test('@smoke creating a note writes its backing file before reload can read it', expect(errors.filter((message) => message.includes('File does not exist'))).toEqual([]) expect(fs.readFileSync(createCalls[0], 'utf8')).toContain('type: Note') }) + +test('@smoke rapid note creation waits for each backing file write', async ({ page }) => { + const errors: string[] = [] + page.on('pageerror', (error) => errors.push(error.message)) + + await dispatchMenuCommandBurst(page, 'file-new-note', 2) + + await expect.poll(() => readProbe(page), { timeout: 7_000 }).toMatchObject({ + createCalls: [ + expect.stringMatching(/untitled-note-\d+\.md$/), + expect.stringMatching(/untitled-note-\d+(?:-\d+)?\.md$/), + ], + getBeforeCreate: [], + maxConcurrentCreates: 1, + startedCreateCalls: [ + expect.stringMatching(/untitled-note-\d+\.md$/), + expect.stringMatching(/untitled-note-\d+(?:-\d+)?\.md$/), + ], + }) + + const { createCalls, getBeforeCreate, maxConcurrentCreates } = await readProbe(page) + expect(new Set(createCalls).size).toBe(2) + expect(getBeforeCreate).toEqual([]) + expect(maxConcurrentCreates).toBe(1) + expect(errors.filter((message) => message.includes('#185') || message.includes('Maximum update depth'))).toEqual([]) +})