diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index fba646d..7a48958 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -66,6 +66,14 @@ export default function App() { const [autosave, setAutosave] = useState(readAutosaveSettings) useEffect(() => subscribeAutosaveSettings(setAutosave), []) + // Provenance: which design the in-memory canvas was loaded as. Differs from + // activeDesignId (the selection) during a switch, so autosave gates on this to + // avoid writing one design's canvas under another's id. Ref mirror for the + // fire-time guard, which must read the live value without re-arming the timer. + const [loadedDesignId, setLoadedDesignId] = useState(null) + const loadedDesignIdRef = useRef(loadedDesignId) + useEffect(() => { loadedDesignIdRef.current = loadedDesignId }, [loadedDesignId]) + const [themeModalOpen, setThemeModalOpen] = useState(false) const [styleEditorType, setStyleEditorType] = useState(null) const [searchOpen, setSearchOpen] = useState(false) @@ -133,9 +141,9 @@ export default function App() { enabled: autosave.enabled, delaySeconds: autosave.delay, hasUnsavedChanges, - activeDesignId, + designId: loadedDesignId, changeSignals: [nodes, edges], - getActiveDesignId: () => useDesignStore.getState().activeDesignId, + getLiveDesignId: () => loadedDesignIdRef.current, onSave: (designId) => { void handleSaveRef.current(designId, { silent: true }) }, }) @@ -168,6 +176,9 @@ export default function App() { } catch { setFloorMap(null) loadCanvas(demoNodes, demoEdges) + } finally { + // Record provenance so autosave writes back under the design just loaded. + setLoadedDesignId(designId ?? null) } }, [loadCanvas, setTheme, setCustomStyle, setFloorMap]) @@ -186,6 +197,8 @@ export default function App() { setFloorMap(null) loadCanvas(demoNodes, demoEdges) } + // Record provenance so autosave writes back under the design just loaded. + setLoadedDesignId(designId) }, [loadCanvas, setTheme, setCustomStyle, setFloorMap]) const loadDesignsAndCanvas = useCallback(async () => { diff --git a/frontend/src/hooks/__tests__/useAutosave.test.ts b/frontend/src/hooks/__tests__/useAutosave.test.ts index a299013..e114d14 100644 --- a/frontend/src/hooks/__tests__/useAutosave.test.ts +++ b/frontend/src/hooks/__tests__/useAutosave.test.ts @@ -6,21 +6,21 @@ interface Args { enabled?: boolean delaySeconds?: number hasUnsavedChanges?: boolean - activeDesignId?: string | null + designId?: string | null changeSignals?: unknown[] - getActiveDesignId?: () => string | null + getLiveDesignId?: () => string | null onSave?: (designId: string) => void } function args(over: Args = {}) { - const activeDesignId = 'activeDesignId' in over ? (over.activeDesignId ?? null) : 'design-a' + const designId = 'designId' in over ? (over.designId ?? null) : 'design-a' return { enabled: over.enabled ?? true, delaySeconds: over.delaySeconds ?? 5, hasUnsavedChanges: over.hasUnsavedChanges ?? true, - activeDesignId, + designId, changeSignals: over.changeSignals ?? [[], []], - getActiveDesignId: over.getActiveDesignId ?? (() => activeDesignId), + getLiveDesignId: over.getLiveDesignId ?? (() => designId), onSave: over.onSave ?? vi.fn(), } } @@ -36,7 +36,8 @@ describe('useAutosave', () => { vi.advanceTimersByTime(4999) expect(onSave).not.toHaveBeenCalled() vi.advanceTimersByTime(1) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) + expect(onSave).toHaveBeenCalledWith('design-a') }) it('does nothing when disabled', () => { @@ -53,9 +54,9 @@ describe('useAutosave', () => { expect(onSave).not.toHaveBeenCalled() }) - it('does nothing when there is no active design', () => { + it('does nothing when the canvas has no known provenance', () => { const onSave = vi.fn() - renderHook(() => useAutosave(args({ onSave, activeDesignId: null }))) + renderHook(() => useAutosave(args({ onSave, designId: null }))) vi.advanceTimersByTime(60_000) expect(onSave).not.toHaveBeenCalled() }) @@ -70,25 +71,27 @@ describe('useAutosave', () => { vi.advanceTimersByTime(4000) expect(onSave).not.toHaveBeenCalled() vi.advanceTimersByTime(1000) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) }) - it('skips the save if the active design changed while the timer was pending', () => { + it('skips the save if a different canvas loaded while the timer was pending', () => { const onSave = vi.fn() - // Pinned at arm time = 'design-a', but live id has moved on to 'design-b'. + // Pinned provenance at arm time = 'design-a', but a switch loaded 'design-b' + // before the timer fired — the on-screen canvas is no longer design-a's. renderHook(() => - useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-b' })), + useAutosave(args({ onSave, designId: 'design-a', getLiveDesignId: () => 'design-b' })), ) vi.advanceTimersByTime(5000) expect(onSave).not.toHaveBeenCalled() }) - it('saves under the pinned design id, not the live one, when they still match', () => { + it('saves under the pinned provenance id when it still matches at fire time', () => { const onSave = vi.fn() renderHook(() => - useAutosave(args({ onSave, activeDesignId: 'design-a', getActiveDesignId: () => 'design-a' })), + useAutosave(args({ onSave, designId: 'design-a', getLiveDesignId: () => 'design-a' })), ) vi.advanceTimersByTime(5000) - expect(onSave).toHaveBeenCalledExactlyOnceWith('design-a') + expect(onSave).toHaveBeenCalledTimes(1) + expect(onSave).toHaveBeenCalledWith('design-a') }) }) diff --git a/frontend/src/hooks/useAutosave.ts b/frontend/src/hooks/useAutosave.ts index e4ac35c..90b43c2 100644 --- a/frontend/src/hooks/useAutosave.ts +++ b/frontend/src/hooks/useAutosave.ts @@ -7,22 +7,28 @@ interface UseAutosaveOptions { delaySeconds: number /** True when the canvas has edits not yet persisted. */ hasUnsavedChanges: boolean - /** The design the in-memory canvas currently belongs to. */ - activeDesignId: string | null + /** + * The design the in-memory canvas actually belongs to — its *provenance*, set + * when a canvas is loaded, NOT the currently-selected design. These differ + * during a design switch: the selection flips synchronously while the new + * canvas loads asynchronously, so for a brief window the on-screen nodes still + * belong to the previous design. Gating on provenance (not selection) is what + * prevents saving one design's canvas under another design's id. + */ + designId: string | null /** * Values that represent canvas edits (e.g. nodes, edges). Any change to one * of these resets the debounce timer, so the save only fires after a quiet - * period. Kept separate from the trigger flags so the caller controls exactly - * what counts as "activity". + * period. Its length MUST stay constant across renders (React requires a + * stable dependency-array size) — pass a fixed-shape array like [nodes, edges]. */ - changeSignals: unknown[] + changeSignals: readonly unknown[] /** - * Reads the *live* active design id at fire time. Used to detect that the user - * switched designs while the timer was pending — if so, the in-memory canvas - * now belongs to a different design and saving it under the pinned id would - * clobber the wrong design, so the save is skipped. + * Reads the *live* canvas provenance at fire time. If it no longer matches the + * id pinned when the timer was armed, a different canvas has since loaded, so + * the save is skipped rather than written under the wrong (now-stale) id. */ - getActiveDesignId: () => string | null + getLiveDesignId: () => string | null /** Persist the canvas under the given design id. */ onSave: (designId: string) => void } @@ -36,30 +42,31 @@ export function useAutosave({ enabled, delaySeconds, hasUnsavedChanges, - activeDesignId, + designId, changeSignals, - getActiveDesignId, + getLiveDesignId, onSave, }: UseAutosaveOptions): void { // Keep the latest callbacks in refs so the timer always calls the current // versions without re-arming (which would reset the debounce) on every render. const onSaveRef = useRef(onSave) - const getActiveDesignIdRef = useRef(getActiveDesignId) + const getLiveDesignIdRef = useRef(getLiveDesignId) useEffect(() => { onSaveRef.current = onSave - getActiveDesignIdRef.current = getActiveDesignId + getLiveDesignIdRef.current = getLiveDesignId }) useEffect(() => { - if (!enabled || !hasUnsavedChanges || !activeDesignId) return - const designId = activeDesignId + if (!enabled || !hasUnsavedChanges || !designId) return + const pinnedDesignId = designId const t = setTimeout(() => { - // Skip if the active design changed while the timer was pending. - if (getActiveDesignIdRef.current() !== designId) return - onSaveRef.current(designId) + // Skip if a different canvas has loaded while the timer was pending. + if (getLiveDesignIdRef.current() !== pinnedDesignId) return + onSaveRef.current(pinnedDesignId) }, delaySeconds * 1000) return () => clearTimeout(t) - // changeSignals is spread so any canvas edit resets the debounce. + // changeSignals is spread so any canvas edit resets the debounce; its length + // must stay constant (documented on the option). // eslint-disable-next-line react-hooks/exhaustive-deps - }, [enabled, delaySeconds, hasUnsavedChanges, activeDesignId, ...changeSignals]) + }, [enabled, delaySeconds, hasUnsavedChanges, designId, ...changeSignals]) } diff --git a/frontend/src/utils/__tests__/autosaveSettings.test.ts b/frontend/src/utils/__tests__/autosaveSettings.test.ts index 3074c92..be31087 100644 --- a/frontend/src/utils/__tests__/autosaveSettings.test.ts +++ b/frontend/src/utils/__tests__/autosaveSettings.test.ts @@ -46,6 +46,11 @@ describe('autosaveSettings', () => { expect(readAutosaveSettings().delay).toBe(DEFAULT_AUTOSAVE_SETTINGS.delay) }) + it('rejects a non-boolean enabled and falls back to default', () => { + localStorage.setItem('homelable.autosave', JSON.stringify({ enabled: 'yes', delay: 10 })) + expect(readAutosaveSettings().enabled).toBe(DEFAULT_AUTOSAVE_SETTINGS.enabled) + }) + it('notifies subscribers on write and stops after unsubscribe', () => { const listener = vi.fn() const unsubscribe = subscribeAutosaveSettings(listener) diff --git a/frontend/src/utils/autosaveSettings.ts b/frontend/src/utils/autosaveSettings.ts index 3b54941..266e0b0 100644 --- a/frontend/src/utils/autosaveSettings.ts +++ b/frontend/src/utils/autosaveSettings.ts @@ -19,7 +19,7 @@ export function readAutosaveSettings(): AutosaveSettings { if (!raw) return DEFAULT_AUTOSAVE_SETTINGS const parsed = JSON.parse(raw) as Partial return { - enabled: parsed.enabled ?? DEFAULT_AUTOSAVE_SETTINGS.enabled, + enabled: typeof parsed.enabled === 'boolean' ? parsed.enabled : DEFAULT_AUTOSAVE_SETTINGS.enabled, delay: typeof parsed.delay === 'number' && parsed.delay > 0 ? parsed.delay : DEFAULT_AUTOSAVE_SETTINGS.delay, } } catch {