Skip to content

Commit e170424

Browse files
committed
fix: address PR review findings — listener leak, type contracts, dead ref, stale guard, cfg scope, shortcut loop, timer constant
1 parent 5289386 commit e170424

10 files changed

Lines changed: 38 additions & 35 deletions

File tree

src-tauri/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,11 @@ tauri-plugin-notification = "2.0.0-rc.5"
3333
cocoa = "0.25"
3434
objc = "0.2"
3535

36+
# The objc v0.2 crate macros use cfg(cargo-clippy) which is no longer recognized,
37+
# producing unexpected_cfgs warnings from external macro expansions that cannot
38+
# be suppressed per-function or per-module (the span originates in objc crate code).
3639
[lints.rust]
3740
unexpected_cfgs = "allow"
41+
42+
43+

src-tauri/src/macos.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(unexpected_cfgs)]
2-
31
#[cfg(target_os = "macos")]
42
use tauri::{AppHandle, Emitter};
53

src/App.tsx

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { Editor, type EditorRef } from './components/Editor'
2424
import Settings from './Settings'
2525

2626
const TOAST_TIMEOUT_MS = 5000
27-
const FOCUS_DELAY_MS = 50
2827
const MODAL_Z_INDEX = 9999
2928
const KEYBINDS_Z_INDEX = 10000
3029
const TOAST_Z_INDEX = 99999
@@ -42,7 +41,6 @@ function App() {
4241
const setShowTimersView = useAppStore((state) => state.setShowTimersView)
4342
const toasts = useAppStore((state) => state.toasts)
4443
const removeToast = useAppStore((state) => state.removeToast)
45-
const showNoteSearch = useAppStore((state) => state.showNoteSearch)
4644
const setShowMainActionMenu = useAppStore((state) => state.setShowMainActionMenu)
4745
const showSettingsModal = useAppStore((state) => state.showSettingsModal)
4846
const setShowSettingsModal = useAppStore((state) => state.setShowSettingsModal)
@@ -54,8 +52,6 @@ function App() {
5452

5553
const editorRef = useRef<EditorRef>(null)
5654

57-
const searchInputRef = useRef<HTMLInputElement>(null)
58-
5955
useNoteStorage()
6056
useVariables()
6157
useReminders()
@@ -120,14 +116,6 @@ function App() {
120116
}
121117
}, [toasts, removeToast])
122118

123-
useEffect(() => {
124-
if (showNoteSearch && searchInputRef.current) {
125-
setTimeout(() => {
126-
searchInputRef.current?.focus()
127-
}, FOCUS_DELAY_MS)
128-
}
129-
}, [showNoteSearch])
130-
131119
useEffect(() => {
132120
async function checkVersion() {
133121
if (notes.length === 0) return

src/api.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,17 @@ import type { ElectronAPI, ReminderPayload } from './types'
44

55
const onEvent = (name: string, callback: () => void) => {
66
let unlisten: (() => void) | undefined
7+
let disposed = false
78
listen(name, () => callback()).then((fn) => {
9+
if (disposed) {
10+
fn()
11+
return
12+
}
813
unlisten = fn
914
})
1015
return () => {
11-
if (unlisten) unlisten()
16+
disposed = true
17+
unlisten?.()
1218
}
1319
}
1420

@@ -55,7 +61,7 @@ export const tauriApi: ElectronAPI = {
5561
safeStorageDecrypt: (val) => invoke('safe_storage_decrypt', { val }),
5662
onPowerSuspend: (callback) => onEvent('power:suspend', callback),
5763
onPowerResume: (callback) => onEvent('power:resume', callback),
58-
pauseShortcuts: () => invoke('pause_shortcuts') as unknown as void,
59-
resumeShortcuts: () => invoke('resume_shortcuts') as unknown as void,
64+
pauseShortcuts: () => invoke('pause_shortcuts'),
65+
resumeShortcuts: () => invoke('resume_shortcuts'),
6066
onUpdateReady: (callback) => onEvent('update-ready', callback),
6167
}

src/components/KeybindsModal.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ interface ShortcutConfig {
1313
storageKey: string
1414
defaultKey: string
1515
section: 'global' | 'app'
16+
action?: string
17+
oldShortcutStorageKey?: string
1618
}
1719

1820
export function KeybindsModal({ onClose }: KeybindsModalProps) {
@@ -26,13 +28,17 @@ export function KeybindsModal({ onClose }: KeybindsModalProps) {
2628
storageKey: SETTINGS_KEYS.SHORTCUT_TOGGLE,
2729
defaultKey: `${defaultMod}+Shift+C`,
2830
section: 'global',
31+
action: 'toggle',
32+
oldShortcutStorageKey: 'papercache-shortcut-toggle',
2933
},
3034
{
3135
key: 'shortcutNewNote',
3236
label: 'New Note (Global)',
3337
storageKey: SETTINGS_KEYS.SHORTCUT_NEWNOTE,
3438
defaultKey: `${defaultMod}+Shift+N`,
3539
section: 'global',
40+
action: 'new-note',
41+
oldShortcutStorageKey: 'papercache-shortcut-newnote',
3642
},
3743
{
3844
key: 'shortcutTasks',
@@ -121,15 +127,12 @@ export function KeybindsModal({ onClose }: KeybindsModalProps) {
121127

122128
const handleSave = () => {
123129
for (const sc of shortcuts) {
124-
if (sc.section === 'global') {
125-
const oldShortcutKey =
126-
sc.key === 'shortcutToggle' ? 'papercache-shortcut-toggle' : 'papercache-shortcut-newnote'
127-
const oldShortcut = localStorage.getItem(oldShortcutKey) || sc.defaultKey
128-
const action = sc.key === 'shortcutToggle' ? 'toggle' : 'new-note'
130+
if (sc.section === 'global' && sc.action && sc.oldShortcutStorageKey) {
131+
const oldShortcut = localStorage.getItem(sc.oldShortcutStorageKey) || sc.defaultKey
129132
if (window.electronAPI.updateGlobalShortcut) {
130-
window.electronAPI.updateGlobalShortcut(action, oldShortcut, values[sc.key])
133+
window.electronAPI.updateGlobalShortcut(sc.action, oldShortcut, values[sc.key])
131134
}
132-
localStorage.setItem(oldShortcutKey, values[sc.key])
135+
localStorage.setItem(sc.oldShortcutStorageKey, values[sc.key])
133136
}
134137
localStorage.setItem(sc.storageKey, values[sc.key])
135138
}

src/components/TimersPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ function TimerItem({ timer, onRemove }: TimerItemProps) {
4141
}
4242
}
4343

44-
timeoutRef.current = setTimeout(tick, 250)
44+
timeoutRef.current = setTimeout(tick, TICK_INTERVAL_MS)
4545

4646
return () => {
4747
if (timeoutRef.current) clearTimeout(timeoutRef.current)

src/hooks/useReminders.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,13 @@ export function useReminders() {
4040
const notes = useAppStore((state) => state.notes)
4141

4242
useEffect(() => {
43-
const pending = collectFutureReminders(notes)
4443
const token = ++scheduleToken
44+
const pending = collectFutureReminders(notes)
45+
if (token !== scheduleToken) return
4546

4647
window.electronAPI
4748
.scheduleReminders(pending)
48-
.then(() => {
49-
if (token !== scheduleToken) return
50-
})
49+
.then(() => {})
5150
// eslint-disable-next-line no-console
5251
.catch((e) => console.error('Failed to schedule reminders', e))
5352
}, [notes])

src/lib/editor/extensions.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,11 @@ export function useEditorExtensions() {
178178
})
179179

180180
let response: string
181-
if (completion.choices && completion.choices.length > 0) {
182-
response = completion.choices[0].message?.content || ''
181+
const choice = completion.choices?.[0]
182+
if (choice?.message?.content) {
183+
response = choice.message.content
184+
} else if (choice?.message?.content === '') {
185+
response = ''
183186
} else if (completion.error) {
184187
throw new Error(completion.error.message || 'Unknown API Error')
185188
} else {

src/setupTests.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ if (typeof window !== 'undefined') {
7373
safeStorageDecrypt: vi.fn((val) => Promise.resolve(val)),
7474
onPowerSuspend: vi.fn().mockReturnValue(() => {}),
7575
onPowerResume: vi.fn().mockReturnValue(() => {}),
76-
pauseShortcuts: vi.fn(),
77-
resumeShortcuts: vi.fn(),
76+
pauseShortcuts: vi.fn().mockResolvedValue(undefined),
77+
resumeShortcuts: vi.fn().mockResolvedValue(undefined),
7878
onUpdateReady: vi.fn().mockReturnValue(() => {}),
7979
scheduleReminders: vi.fn().mockResolvedValue(undefined),
8080
cancelReminders: vi.fn().mockResolvedValue(undefined),

src/types.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ export interface ElectronAPI {
4444
safeStorageDecrypt: (val: string) => Promise<string>
4545
onPowerSuspend: (callback: () => void) => () => void
4646
onPowerResume: (callback: () => void) => () => void
47-
pauseShortcuts: () => void
48-
resumeShortcuts: () => void
47+
pauseShortcuts: () => Promise<void>
48+
resumeShortcuts: () => Promise<void>
4949
onUpdateReady: (callback: () => void) => () => void
5050
}
5151

0 commit comments

Comments
 (0)