fix: replace broken auto-save with explicit Cmd+S save

Removes the debounced auto-save (useAutoSave hook) which was causing the
editor to reload previous content. Replaces it with explicit save on
Cmd+S (⌘S), consistent with the git-based UX of the app.

- Removed useAutoSave hook and its debounce mechanism
- Added useSaveNote hook for direct persist-to-disk
- Added useEditorSave hook that manages pending content buffer + save
- Cmd+S now persists the current editor content immediately
- Rename-before-save: saves pending content before rename to prevent
  the "Failed to rename note" error
- Updated E2E test to verify Cmd+S behavior
- 471 tests passing, code health gates passed

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
lucaronin
2026-02-23 19:53:36 +01:00
parent d5fbff07ad
commit 6c2e1b7607
10 changed files with 322 additions and 243 deletions

View File

@@ -8,7 +8,7 @@ test.beforeEach(async ({ page }) => {
})
test('editor loads and renders note content for editing', async ({ page }) => {
await page.screenshot({ path: 'test-results/auto-save-01-initial.png', fullPage: true })
await page.screenshot({ path: 'test-results/save-01-initial.png', fullPage: true })
// 1. Click a note in the note list panel
const noteList = page.locator('.app__note-list')
@@ -26,7 +26,7 @@ test('editor loads and renders note content for editing', async ({ page }) => {
const isEditable = await editor.getAttribute('contenteditable')
expect(isEditable).toBe('true')
await page.screenshot({ path: 'test-results/auto-save-02-note-open.png', fullPage: true })
await page.screenshot({ path: 'test-results/save-02-note-open.png', fullPage: true })
// 3. Verify the editor has content (not empty)
const editorText = await page.evaluate(() => {
@@ -39,5 +39,30 @@ test('editor loads and renders note content for editing', async ({ page }) => {
const tabBar = page.locator('.editor')
await expect(tabBar).toBeVisible()
await page.screenshot({ path: 'test-results/auto-save-03-editor-ready.png', fullPage: true })
await page.screenshot({ path: 'test-results/save-03-editor-ready.png', fullPage: true })
})
test('Cmd+S triggers explicit save and shows toast', async ({ page }) => {
// Open a note
const noteList = page.locator('.app__note-list')
await expect(noteList).toBeVisible({ timeout: 5000 })
const firstNote = noteList.locator('div.cursor-pointer').first()
await firstNote.click()
await page.waitForTimeout(1000)
// Type some content to make the editor dirty
const editor = page.locator('.bn-editor')
await editor.click()
await page.keyboard.type('Hello from Cmd+S test')
await page.waitForTimeout(300)
// Press Cmd+S
await page.keyboard.press('Meta+s')
await page.waitForTimeout(500)
// Verify toast appears with "Saved" message
const toast = page.locator('text=Saved')
await expect(toast).toBeVisible({ timeout: 3000 })
await page.screenshot({ path: 'test-results/save-04-cmd-s-saved.png', fullPage: true })
})

View File

@@ -143,10 +143,10 @@ describe('App', () => {
expect(screen.getByText('All Notes')).toBeInTheDocument()
})
// Cmd+S shows toast
// Cmd+S with no pending changes shows "Nothing to save"
fireEvent.keyDown(window, { key: 's', metaKey: true })
await waitFor(() => {
expect(screen.getByText('Saved')).toBeInTheDocument()
expect(screen.getByText('Nothing to save')).toBeInTheDocument()
})
})

View File

@@ -13,7 +13,7 @@ import { GitHubVaultModal } from './components/GitHubVaultModal'
import { useVaultLoader } from './hooks/useVaultLoader'
import { useSettings } from './hooks/useSettings'
import { useNoteActions, generateUntitledName } from './hooks/useNoteActions'
import { useAutoSave } from './hooks/useAutoSave'
import { useEditorSave } from './hooks/useEditorSave'
import { useAppKeyboard } from './hooks/useAppKeyboard'
import { useViewMode } from './hooks/useViewMode'
import { useEntryActions } from './hooks/useEntryActions'
@@ -81,14 +81,11 @@ function App() {
const notes = useNoteActions({ addEntry: vault.addEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry })
const updateTabAndContent = useCallback((path: string, content: string) => {
vault.updateContent(path, content)
notes.setTabs((prev) =>
prev.map((t) => t.entry.path === path ? { ...t, content } : t)
)
}, [vault, notes])
const { debouncedSave, flush: flushSave } = useAutoSave(updateTabAndContent)
const { handleSave, handleContentChange, savePendingForPath } = useEditorSave({
updateVaultContent: vault.updateContent,
setTabs: notes.setTabs,
setToastMessage,
})
const entryActions = useEntryActions({
entries: vault.entries,
@@ -136,16 +133,18 @@ function App() {
setToastMessage(`Type "${name}" created`)
}, [notes])
const handleRenameTab = useCallback((path: string, newTitle: string) => {
const handleRenameTab = useCallback(async (path: string, newTitle: string) => {
// Save any pending content before renaming so the file on disk is up to date
await savePendingForPath(path)
notes.handleRenameNote(path, newTitle, vaultPath, vault.replaceEntry)
}, [notes, vaultPath, vault])
}, [notes, vaultPath, vault, savePendingForPath])
const { setViewMode } = useViewMode()
useAppKeyboard({
onQuickOpen: () => setShowQuickOpen(true),
onCreateNote: handleCreateNoteImmediate,
onSave: () => setToastMessage('Saved'),
onSave: handleSave,
onOpenSettings: () => setShowSettings(true),
onTrashNote: entryActions.handleTrashNote,
onArchiveNote: entryActions.handleArchiveNote,
@@ -225,8 +224,7 @@ function App() {
onArchiveNote={entryActions.handleArchiveNote}
onUnarchiveNote={entryActions.handleUnarchiveNote}
onRenameTab={handleRenameTab}
onContentChange={debouncedSave}
onFlushSave={flushSave}
onContentChange={handleContentChange}
/>
</div>
</div>

View File

@@ -56,7 +56,6 @@ interface EditorProps {
onUnarchiveNote?: (path: string) => void
onRenameTab?: (path: string, newTitle: string) => void
onContentChange?: (path: string, content: string) => void
onFlushSave?: () => void
}
// --- Custom Inline Content: WikiLink ---
@@ -205,7 +204,6 @@ export const Editor = memo(function Editor({
onArchiveNote, onUnarchiveNote,
onRenameTab,
onContentChange,
onFlushSave,
}: EditorProps) {
const [diffMode, setDiffMode] = useState(false)
const [diffContent, setDiffContent] = useState<string | null>(null)
@@ -269,7 +267,7 @@ export const Editor = memo(function Editor({
return cleanup
}, [editor])
// Suppress auto-save during programmatic content swaps (tab switching / initial load)
// Suppress onChange during programmatic content swaps (tab switching / initial load)
const suppressChangeRef = useRef(false)
// Keep refs to callbacks for the onChange handler
@@ -310,8 +308,6 @@ export const Editor = memo(function Editor({
// Save current editor state for the tab we're leaving
if (prevPath && prevPath !== activeTabPath && editorMountedRef.current) {
cache.set(prevPath, editor.document)
// Flush any pending debounced save for the tab we're leaving
onFlushSave?.()
}
prevActivePathRef.current = activeTabPath

View File

@@ -1,152 +0,0 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { renderHook, act } from '@testing-library/react'
import { useAutoSave } from './useAutoSave'
const mockInvokeFn = vi.fn<(cmd: string, args?: Record<string, unknown>) => Promise<null>>(() => Promise.resolve(null))
vi.mock('@tauri-apps/api/core', () => ({
invoke: vi.fn(),
}))
vi.mock('../mock-tauri', () => ({
isTauri: () => false,
mockInvoke: (cmd: string, args?: Record<string, unknown>) => mockInvokeFn(cmd, args),
updateMockContent: vi.fn(),
}))
describe('useAutoSave', () => {
let updateContent: (path: string, content: string) => void
beforeEach(() => {
vi.useFakeTimers()
updateContent = vi.fn<(path: string, content: string) => void>()
mockInvokeFn.mockClear()
})
afterEach(() => {
vi.useRealTimers()
})
it('debounces save calls by 500ms', async () => {
const { result } = renderHook(() => useAutoSave(updateContent))
act(() => {
result.current.debouncedSave('/test/note.md', 'content v1')
})
// Not saved yet
expect(mockInvokeFn).not.toHaveBeenCalled()
expect(updateContent).not.toHaveBeenCalled()
// Advance 300ms — still not saved
act(() => { vi.advanceTimersByTime(300) })
expect(mockInvokeFn).not.toHaveBeenCalled()
// Advance to 500ms — now saved
await act(async () => { vi.advanceTimersByTime(200) })
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note.md',
content: 'content v1',
})
expect(updateContent).toHaveBeenCalledWith('/test/note.md', 'content v1')
})
it('only saves the latest content when debounce resets', async () => {
const { result } = renderHook(() => useAutoSave(updateContent))
act(() => {
result.current.debouncedSave('/test/note.md', 'version 1')
})
act(() => { vi.advanceTimersByTime(200) })
// Second call resets the timer
act(() => {
result.current.debouncedSave('/test/note.md', 'version 2')
})
// Advance past original deadline
act(() => { vi.advanceTimersByTime(300) })
expect(mockInvokeFn).not.toHaveBeenCalled()
// Advance to new deadline (200 + 500 = 700ms from second call)
await act(async () => { vi.advanceTimersByTime(200) })
expect(mockInvokeFn).toHaveBeenCalledTimes(1)
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note.md',
content: 'version 2',
})
})
it('flush() immediately saves pending content', async () => {
const { result } = renderHook(() => useAutoSave(updateContent))
act(() => {
result.current.debouncedSave('/test/note.md', 'flush me')
})
expect(mockInvokeFn).not.toHaveBeenCalled()
// Flush before debounce fires
await act(async () => {
result.current.flush()
})
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note.md',
content: 'flush me',
})
})
it('saves multiple paths when switching notes rapidly', async () => {
const { result } = renderHook(() => useAutoSave(updateContent))
act(() => {
result.current.debouncedSave('/test/note-a.md', 'content A')
result.current.debouncedSave('/test/note-b.md', 'content B')
})
await act(async () => { vi.advanceTimersByTime(500) })
expect(mockInvokeFn).toHaveBeenCalledTimes(2)
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note-a.md',
content: 'content A',
})
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note-b.md',
content: 'content B',
})
})
it('handles save errors gracefully without crashing', async () => {
mockInvokeFn.mockRejectedValueOnce(new Error('File is read-only'))
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const { result } = renderHook(() => useAutoSave(updateContent))
act(() => {
result.current.debouncedSave('/test/readonly.md', 'content')
})
await act(async () => { vi.advanceTimersByTime(500) })
expect(consoleSpy).toHaveBeenCalledWith(
expect.stringContaining('Auto-save failed'),
expect.any(Error),
)
consoleSpy.mockRestore()
})
it('does nothing when flush is called with no pending saves', async () => {
const { result } = renderHook(() => useAutoSave(updateContent))
await act(async () => {
result.current.flush()
})
expect(mockInvokeFn).not.toHaveBeenCalled()
expect(updateContent).not.toHaveBeenCalled()
})
})

View File

@@ -1,67 +0,0 @@
import { useCallback, useRef } from 'react'
import { invoke } from '@tauri-apps/api/core'
import { isTauri, mockInvoke, updateMockContent } from '../mock-tauri'
const DEBOUNCE_MS = 500
async function persistContent(path: string, content: string): Promise<void> {
if (isTauri()) {
await invoke('save_note_content', { path, content })
} else {
await mockInvoke('save_note_content', { path, content })
}
}
/**
* Hook that provides a debounced auto-save function for note content.
* Calls the Tauri backend (or mock) to persist the full markdown (frontmatter + body)
* after 500ms of inactivity.
*
* @param updateContent - callback to also update in-memory allContent state
*/
export function useAutoSave(updateContent: (path: string, content: string) => void) {
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
// Track the latest pending content per path so a flush always saves the most recent version
const pendingRef = useRef<Map<string, string>>(new Map())
const save = useCallback(async (path: string, content: string) => {
try {
await persistContent(path, content)
if (!isTauri()) {
updateMockContent(path, content)
}
updateContent(path, content)
} catch (err) {
console.error(`Auto-save failed for ${path}:`, err)
}
}, [updateContent])
const debouncedSave = useCallback((path: string, content: string) => {
pendingRef.current.set(path, content)
if (timerRef.current) clearTimeout(timerRef.current)
timerRef.current = setTimeout(() => {
timerRef.current = null
// Save all pending paths (handles rapid note switching)
const pending = new Map(pendingRef.current)
pendingRef.current.clear()
for (const [p, c] of pending) {
save(p, c)
}
}, DEBOUNCE_MS)
}, [save])
/** Immediately flush any pending saves (call before closing a tab or switching notes) */
const flush = useCallback(() => {
if (timerRef.current) {
clearTimeout(timerRef.current)
timerRef.current = null
}
const pending = new Map(pendingRef.current)
pendingRef.current.clear()
for (const [p, c] of pending) {
save(p, c)
}
}, [save])
return { debouncedSave, flush }
}

View File

@@ -0,0 +1,121 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { renderHook, act } from '@testing-library/react'
import { useEditorSave } from './useEditorSave'
const mockInvokeFn = vi.fn<(cmd: string, args?: Record<string, unknown>) => Promise<null>>(() => Promise.resolve(null))
vi.mock('@tauri-apps/api/core', () => ({
invoke: vi.fn(),
}))
vi.mock('../mock-tauri', () => ({
isTauri: () => false,
mockInvoke: (cmd: string, args?: Record<string, unknown>) => mockInvokeFn(cmd, args),
updateMockContent: vi.fn(),
}))
describe('useEditorSave', () => {
let updateVaultContent: vi.Mock
let setTabs: vi.Mock
let setToastMessage: vi.Mock
beforeEach(() => {
updateVaultContent = vi.fn()
setTabs = vi.fn()
setToastMessage = vi.fn()
mockInvokeFn.mockClear()
})
function renderSaveHook() {
return renderHook(() => useEditorSave({ updateVaultContent, setTabs, setToastMessage }))
}
it('handleSave shows "Nothing to save" when no pending content', async () => {
const { result } = renderSaveHook()
await act(async () => {
await result.current.handleSave()
})
expect(setToastMessage).toHaveBeenCalledWith('Nothing to save')
expect(mockInvokeFn).not.toHaveBeenCalled()
})
it('handleSave persists pending content and shows "Saved"', async () => {
const { result } = renderSaveHook()
// Buffer content via handleContentChange
act(() => {
result.current.handleContentChange('/test/note.md', '---\ntitle: Test\n---\n\n# Test\n\nEdited')
})
// Save via Cmd+S
await act(async () => {
await result.current.handleSave()
})
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note.md',
content: '---\ntitle: Test\n---\n\n# Test\n\nEdited',
})
expect(setToastMessage).toHaveBeenCalledWith('Saved')
// Second save should show "Nothing to save" (pending cleared)
await act(async () => {
await result.current.handleSave()
})
expect(setToastMessage).toHaveBeenCalledWith('Nothing to save')
})
it('handleSave shows error toast on failure', async () => {
mockInvokeFn.mockRejectedValueOnce(new Error('Disk full'))
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const { result } = renderSaveHook()
act(() => {
result.current.handleContentChange('/test/note.md', 'content')
})
await act(async () => {
await result.current.handleSave()
})
expect(setToastMessage).toHaveBeenCalledWith(expect.stringContaining('Save failed'))
consoleSpy.mockRestore()
})
it('savePendingForPath saves content only for the matching path', async () => {
const { result } = renderSaveHook()
act(() => {
result.current.handleContentChange('/test/note-a.md', 'content A')
})
// Try saving for a different path — should be a no-op
await act(async () => {
await result.current.savePendingForPath('/test/note-b.md')
})
expect(mockInvokeFn).not.toHaveBeenCalled()
// Save for the correct path
await act(async () => {
await result.current.savePendingForPath('/test/note-a.md')
})
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note-a.md',
content: 'content A',
})
})
it('handleContentChange buffers the latest content', () => {
const { result } = renderSaveHook()
act(() => {
result.current.handleContentChange('/test/note.md', 'v1')
result.current.handleContentChange('/test/note.md', 'v2')
})
// The ref should hold the latest value — verified via save
// (We'll check via the next handleSave call)
})
})

View File

@@ -0,0 +1,65 @@
import { useCallback, useRef } from 'react'
import type { SetStateAction } from 'react'
import { useSaveNote } from './useSaveNote'
interface Tab {
entry: { path: string }
content: string
}
interface EditorSaveConfig {
updateVaultContent: (path: string, content: string) => void
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Tab types vary between layers
setTabs: (fn: SetStateAction<any[]>) => void
setToastMessage: (msg: string | null) => void
}
/**
* Hook that manages explicit save (Cmd+S) for editor content.
* Tracks pending (unsaved) content and provides save + pre-rename helpers.
*/
export function useEditorSave({ updateVaultContent, setTabs, setToastMessage }: EditorSaveConfig) {
const pendingContentRef = useRef<{ path: string; content: string } | null>(null)
const updateTabAndContent = useCallback((path: string, content: string) => {
updateVaultContent(path, content)
setTabs((prev: Tab[]) =>
prev.map((t) => t.entry.path === path ? { ...t, content } : t)
)
}, [updateVaultContent, setTabs])
const { saveNote } = useSaveNote(updateTabAndContent)
/** Called by Cmd+S — persists the current editor content to disk */
const handleSave = useCallback(async () => {
const pending = pendingContentRef.current
if (!pending) {
setToastMessage('Nothing to save')
return
}
try {
await saveNote(pending.path, pending.content)
pendingContentRef.current = null
setToastMessage('Saved')
} catch (err) {
console.error('Save failed:', err)
setToastMessage(`Save failed: ${err}`)
}
}, [saveNote, setToastMessage])
/** Called by Editor onChange — buffers the latest content without saving */
const handleContentChange = useCallback((path: string, content: string) => {
pendingContentRef.current = { path, content }
}, [])
/** Save pending content for a specific path (used before rename) */
const savePendingForPath = useCallback(async (path: string): Promise<void> => {
const pending = pendingContentRef.current
if (pending && pending.path === path) {
await saveNote(pending.path, pending.content)
pendingContentRef.current = null
}
}, [saveNote])
return { handleSave, handleContentChange, savePendingForPath }
}

View File

@@ -0,0 +1,64 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { renderHook, act } from '@testing-library/react'
import { useSaveNote } from './useSaveNote'
const mockInvokeFn = vi.fn<(cmd: string, args?: Record<string, unknown>) => Promise<null>>(() => Promise.resolve(null))
vi.mock('@tauri-apps/api/core', () => ({
invoke: vi.fn(),
}))
vi.mock('../mock-tauri', () => ({
isTauri: () => false,
mockInvoke: (cmd: string, args?: Record<string, unknown>) => mockInvokeFn(cmd, args),
updateMockContent: vi.fn(),
}))
describe('useSaveNote', () => {
let updateContent: (path: string, content: string) => void
beforeEach(() => {
updateContent = vi.fn<(path: string, content: string) => void>()
mockInvokeFn.mockClear()
})
it('saves content immediately via Tauri command', async () => {
const { result } = renderHook(() => useSaveNote(updateContent))
await act(async () => {
await result.current.saveNote('/test/note.md', '---\ntitle: Test\n---\n\n# Test\n\nContent')
})
expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', {
path: '/test/note.md',
content: '---\ntitle: Test\n---\n\n# Test\n\nContent',
})
expect(updateContent).toHaveBeenCalledWith('/test/note.md', '---\ntitle: Test\n---\n\n# Test\n\nContent')
})
it('updates in-memory state after saving', async () => {
const { result } = renderHook(() => useSaveNote(updateContent))
await act(async () => {
await result.current.saveNote('/test/a.md', 'content A')
await result.current.saveNote('/test/b.md', 'content B')
})
expect(updateContent).toHaveBeenCalledTimes(2)
expect(updateContent).toHaveBeenCalledWith('/test/a.md', 'content A')
expect(updateContent).toHaveBeenCalledWith('/test/b.md', 'content B')
})
it('propagates save errors to the caller', async () => {
mockInvokeFn.mockRejectedValueOnce(new Error('File is read-only'))
const { result } = renderHook(() => useSaveNote(updateContent))
await expect(
act(async () => {
await result.current.saveNote('/test/readonly.md', 'content')
})
).rejects.toThrow('File is read-only')
expect(updateContent).not.toHaveBeenCalled()
})
})

29
src/hooks/useSaveNote.ts Normal file
View File

@@ -0,0 +1,29 @@
import { useCallback } from 'react'
import { invoke } from '@tauri-apps/api/core'
import { isTauri, mockInvoke, updateMockContent } from '../mock-tauri'
async function persistContent(path: string, content: string): Promise<void> {
if (isTauri()) {
await invoke('save_note_content', { path, content })
} else {
await mockInvoke('save_note_content', { path, content })
}
}
/**
* Hook that provides an explicit save function for note content.
* Called on Cmd+S — no debounce, no auto-save.
*
* @param updateContent - callback to also update in-memory state after save
*/
export function useSaveNote(updateContent: (path: string, content: string) => void) {
const saveNote = useCallback(async (path: string, content: string) => {
await persistContent(path, content)
if (!isTauri()) {
updateMockContent(path, content)
}
updateContent(path, content)
}, [updateContent])
return { saveNote }
}