From 12ee4cbf3bb3debc86e4dd73c0d543c5525dc31d Mon Sep 17 00:00:00 2001 From: Kaan Karaca Date: Sat, 8 Aug 2026 04:23:02 +0300 Subject: [PATCH] perf(tabs): serialize the tab tree once per debounce window The auto-save effect ran serializeTabState() plus JSON.stringify() on every tab-state change and only debounced the write, so a burst of changes walked the whole tab tree and JSON-encoded the payload once per change while producing a single save. Dragging a split divider dispatches RESIZE_SPLIT on every mousemove, so this ran at pointer-event frequency. Move both calls inside the debounced callback. The effect still re-runs on every state change, so only the last timer of a burst survives and it closes over the newest state - the persisted payload is unchanged. Also register the beforeunload listener once instead of tearing it down and re-adding it on every state change; it now reads the current state from the ref the flush registry already keeps in sync. --- .../src/contexts/tabs/persistence/hooks.ts | 17 +-- .../tabs/persistence/persistence.test.tsx | 117 ++++++++++++++++++ 2 files changed, 127 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/renderer/src/contexts/tabs/persistence/hooks.ts b/apps/desktop/src/renderer/src/contexts/tabs/persistence/hooks.ts index f198cd3d1..f65467868 100644 --- a/apps/desktop/src/renderer/src/contexts/tabs/persistence/hooks.ts +++ b/apps/desktop/src/renderer/src/contexts/tabs/persistence/hooks.ts @@ -62,16 +62,19 @@ export const useTabPersistence = (options: UseTabPersistenceOptions = {}): void useEffect(() => { if (!enabled) return - const serialized = serializeTabState(state) - const json = JSON.stringify(serialized) - - if (json === lastSavedRef.current) return - if (saveTimeoutRef.current) { clearTimeout(saveTimeoutRef.current) } + // Serialize inside the debounced callback: only the last timer of a burst + // survives, and it closes over the newest state, so a burst of tab-state + // changes walks the tab tree once instead of once per change. saveTimeoutRef.current = setTimeout(() => { + const serialized = serializeTabState(state) + const json = JSON.stringify(serialized) + + if (json === lastSavedRef.current) return + void storage.save(serialized).then(() => { lastSavedRef.current = json }) @@ -89,13 +92,13 @@ export const useTabPersistence = (options: UseTabPersistenceOptions = {}): void if (!enabled) return const handleBeforeUnload = (): void => { - const serialized = serializeTabState(state) + const serialized = serializeTabState(stateRef.current) saveSync(serialized) } window.addEventListener('beforeunload', handleBeforeUnload) return () => window.removeEventListener('beforeunload', handleBeforeUnload) - }, [state, enabled]) + }, [enabled]) } // ============================================================================= diff --git a/apps/desktop/src/renderer/src/contexts/tabs/persistence/persistence.test.tsx b/apps/desktop/src/renderer/src/contexts/tabs/persistence/persistence.test.tsx index 5aea7a368..cc4bc9c39 100644 --- a/apps/desktop/src/renderer/src/contexts/tabs/persistence/persistence.test.tsx +++ b/apps/desktop/src/renderer/src/contexts/tabs/persistence/persistence.test.tsx @@ -12,12 +12,26 @@ const mocks = vi.hoisted(() => ({ tabsState: null as TabSystemState | null, dispatch: vi.fn(), pendingSave: null as null | (() => void), + serializeCalls: 0, registerPendingSave: vi.fn((_: string, callback: () => void) => { mocks.pendingSave = callback }), unregisterPendingSave: vi.fn() })) +// Counts full tab-tree serializations so the auto-save tests can assert how many +// times the tree is walked, independently of how long the debounce waits. +vi.mock('./serialization', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + serializeTabState: (state: TabSystemState) => { + mocks.serializeCalls += 1 + return actual.serializeTabState(state) + } + } +}) + vi.mock('@/contexts/tabs', () => ({ useTabs: () => ({ state: mocks.tabsState, @@ -137,6 +151,59 @@ const persisted = (overrides: Partial = {}): PersistedTabStat ...overrides }) +const CLOCK_START = 1_700_000_000_000 +const STATE_CHANGES = 20 + +/** Publishes 20 distinct tab states, re-rendering the probe after each one. */ +function applyStateChanges(rerender: () => void): void { + for (let i = 1; i <= STATE_CHANGES; i++) { + const next = state() + next.tabGroups['group-1'].tabs[0] = tab({ title: `Roadmap ${i}` }) + mocks.tabsState = next + rerender() + } +} + +/** The exact payload the last of those states must serialize to. */ +const expectedPersisted = (savedAt: number): PersistedTabState => ({ + version: 2, + tabGroups: { + 'group-1': { + id: 'group-1', + activeTabId: 'tab-1', + tabs: [ + { + id: 'tab-1', + type: 'note', + title: `Roadmap ${STATE_CHANGES}`, + icon: 'file-text', + emoji: null, + path: '/note/tab-1', + entityId: 'tab-1', + isPinned: false, + scrollPosition: 42, + viewState: { cursor: 'top' } + }, + { + id: 'pin-1', + type: 'inbox', + title: 'Inbox', + icon: 'inbox', + emoji: null, + path: '/inbox', + isPinned: true, + scrollPosition: 42, + viewState: { cursor: 'top' } + } + ] + } + }, + layout: { type: 'leaf', tabGroupId: 'group-1' }, + activeGroupId: 'group-1', + settings: state().settings, + savedAt +}) + function withQueryClient(children: React.ReactNode) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } @@ -320,6 +387,7 @@ describe('tab persistence hooks', () => { localStorage.clear() mocks.tabsState = state() mocks.pendingSave = null + mocks.serializeCalls = 0 sessionSnapshot = null manualSnapshot = null }) @@ -356,6 +424,55 @@ describe('tab persistence hooks', () => { expect(mocks.unregisterPendingSave).toHaveBeenCalledWith('tab-state') }) + it('serializes once per debounce window, not once per tab-state change', async () => { + const storage: TabStorage = { + save: vi.fn().mockResolvedValue(undefined), + load: vi.fn(), + clear: vi.fn() + } + vi.setSystemTime(CLOCK_START) + + const { rerender } = render() + applyStateChanges(() => rerender()) + + // Mount plus 20 tab-state changes: the tree must not be walked once per change. + expect(mocks.serializeCalls).toBe(0) + expect(storage.save).not.toHaveBeenCalled() + + act(() => vi.advanceTimersByTime(25)) + + expect(mocks.serializeCalls).toBe(1) + await waitFor(() => expect(storage.save).toHaveBeenCalledTimes(1)) + expect(storage.save).toHaveBeenCalledWith(expectedPersisted(CLOCK_START + 25)) + }) + + it('registers the unload handler once and writes the latest state on unload', () => { + const storage: TabStorage = { + save: vi.fn().mockResolvedValue(undefined), + load: vi.fn(), + clear: vi.fn() + } + const addSpy = vi.spyOn(window, 'addEventListener') + const removeSpy = vi.spyOn(window, 'removeEventListener') + vi.setSystemTime(CLOCK_START) + + const { rerender } = render() + applyStateChanges(() => rerender()) + + const unloadAdds = addSpy.mock.calls.filter(([type]) => type === 'beforeunload') + const unloadRemovals = removeSpy.mock.calls.filter(([type]) => type === 'beforeunload') + expect(unloadAdds).toHaveLength(1) + expect(unloadRemovals).toHaveLength(0) + + act(() => window.dispatchEvent(new Event('beforeunload'))) + + // The one long-lived handler must still write the newest tab tree, not the + // tree it closed over when it was registered. + expect(JSON.parse(localStorage.getItem(STORAGE_KEY) ?? 'null')).toEqual( + expectedPersisted(CLOCK_START) + ) + }) + it('restores full sessions, pinned-only sessions, errors, and manual operations', async () => { const storage: TabStorage = { save: vi.fn().mockResolvedValue(undefined),