diff --git a/desktop/src/features/settings/ui/SpeechPlaybackSettings.tsx b/desktop/src/features/settings/ui/SpeechPlaybackSettings.tsx index 7bbc890d55..e09f354862 100644 --- a/desktop/src/features/settings/ui/SpeechPlaybackSettings.tsx +++ b/desktop/src/features/settings/ui/SpeechPlaybackSettings.tsx @@ -6,6 +6,7 @@ import { import { Button } from "@/shared/ui/button"; import { applyLoadedPlaybackSpeed, + cancelPlaybackSpeedPersistence, commitPlaybackSpeed, } from "./playbackSpeedPersistence"; import { SettingsOptionGroup, SettingsOptionRow } from "./SettingsOptionGroup"; @@ -19,18 +20,21 @@ export function SpeechPlaybackSettings() { const [speed, setSpeed] = useState(DEFAULT_SPEED); const [error, setError] = useState(null); const [loaded, setLoaded] = useState(false); + const active = useRef(true); const confirmedSpeed = useRef(DEFAULT_SPEED); const desiredSpeed = useRef(DEFAULT_SPEED); const flushPromise = useRef | null>(null); const hasLocalIntent = useRef(false); useEffect(() => { - let active = true; + active.current = true; + let loadActive = true; getTtsPlaybackSpeed() .then((savedSpeed) => { - if (active) { + if (loadActive) { applyLoadedPlaybackSpeed( { + active, confirmedSpeed, desiredSpeed, flushPromise, @@ -42,19 +46,32 @@ export function SpeechPlaybackSettings() { } }) .catch((cause) => { - if (active) setError(String(cause)); + if (loadActive) setError(String(cause)); }) .finally(() => { - if (active) setLoaded(true); + if (loadActive) setLoaded(true); }); return () => { - active = false; + loadActive = false; + cancelPlaybackSpeedPersistence({ + active, + confirmedSpeed, + desiredSpeed, + flushPromise, + hasLocalIntent, + }); }; }, []); const commitSpeed = (nextSpeed: number) => { commitPlaybackSpeed( - { confirmedSpeed, desiredSpeed, flushPromise, hasLocalIntent }, + { + active, + confirmedSpeed, + desiredSpeed, + flushPromise, + hasLocalIntent, + }, { persist: setTtsPlaybackSpeed, setError, setSpeed }, nextSpeed, ); diff --git a/desktop/src/features/settings/ui/playbackSpeedPersistence.test.mjs b/desktop/src/features/settings/ui/playbackSpeedPersistence.test.mjs index 779a22c2ce..3152a01034 100644 --- a/desktop/src/features/settings/ui/playbackSpeedPersistence.test.mjs +++ b/desktop/src/features/settings/ui/playbackSpeedPersistence.test.mjs @@ -3,6 +3,7 @@ import test from "node:test"; import { applyLoadedPlaybackSpeed, + cancelPlaybackSpeedPersistence, commitPlaybackSpeed, } from "./playbackSpeedPersistence.ts"; @@ -22,6 +23,7 @@ function harness() { const saves = []; const speeds = []; const state = { + active: { current: true }, confirmedSpeed: { current: 1 }, desiredSpeed: { current: 1 }, flushPromise: { current: null }, @@ -104,3 +106,23 @@ test("a delayed initial load cannot overwrite a local intent", async () => { assert.equal(h.state.confirmedSpeed.current, 1.5); assert.equal(h.state.desiredSpeed.current, 1.5); }); + +test("an unmounted instance cannot persist stale queued intent", async () => { + const oldInstance = harness(); + commitPlaybackSpeed(oldInstance.state, oldInstance.callbacks, 1.25); + commitPlaybackSpeed(oldInstance.state, oldInstance.callbacks, 1.5); + cancelPlaybackSpeedPersistence(oldInstance.state); + + const newInstance = harness(); + commitPlaybackSpeed(newInstance.state, newInstance.callbacks, 0.75); + assert.deepEqual(oldInstance.calls, [1.25]); + assert.deepEqual(newInstance.calls, [0.75]); + + oldInstance.saves[0].resolve(); + await settle(oldInstance.state); + assert.deepEqual(oldInstance.calls, [1.25]); + + newInstance.saves[0].resolve(); + await settle(newInstance.state); + assert.equal(newInstance.state.confirmedSpeed.current, 0.75); +}); diff --git a/desktop/src/features/settings/ui/playbackSpeedPersistence.ts b/desktop/src/features/settings/ui/playbackSpeedPersistence.ts index 88240888e3..5f926f2bea 100644 --- a/desktop/src/features/settings/ui/playbackSpeedPersistence.ts +++ b/desktop/src/features/settings/ui/playbackSpeedPersistence.ts @@ -3,6 +3,7 @@ type MutableValue = { }; export type PlaybackSpeedPersistenceState = { + active: MutableValue; confirmedSpeed: MutableValue; desiredSpeed: MutableValue; flushPromise: MutableValue | null>; @@ -20,6 +21,7 @@ export function commitPlaybackSpeed( callbacks: PlaybackSpeedPersistenceCallbacks, nextSpeed: number, ) { + if (!state.active.current) return; state.hasLocalIntent.current = true; state.desiredSpeed.current = nextSpeed; callbacks.setSpeed(nextSpeed); @@ -27,6 +29,12 @@ export function commitPlaybackSpeed( ensurePlaybackSpeedFlush(state, callbacks); } +export function cancelPlaybackSpeedPersistence( + state: PlaybackSpeedPersistenceState, +) { + state.active.current = false; +} + export function applyLoadedPlaybackSpeed( state: PlaybackSpeedPersistenceState, callbacks: Pick, @@ -42,12 +50,15 @@ function ensurePlaybackSpeedFlush( state: PlaybackSpeedPersistenceState, callbacks: PlaybackSpeedPersistenceCallbacks, ) { - if (state.flushPromise.current) return; + if (!state.active.current || state.flushPromise.current) return; const flush = flushPlaybackSpeed(state, callbacks).finally(() => { if (state.flushPromise.current !== flush) return; state.flushPromise.current = null; - if (state.desiredSpeed.current !== state.confirmedSpeed.current) { + if ( + state.active.current && + state.desiredSpeed.current !== state.confirmedSpeed.current + ) { ensurePlaybackSpeedFlush(state, callbacks); } }); @@ -58,12 +69,17 @@ async function flushPlaybackSpeed( state: PlaybackSpeedPersistenceState, callbacks: PlaybackSpeedPersistenceCallbacks, ) { - while (state.desiredSpeed.current !== state.confirmedSpeed.current) { + while ( + state.active.current && + state.desiredSpeed.current !== state.confirmedSpeed.current + ) { const target = state.desiredSpeed.current; try { await callbacks.persist(target); + if (!state.active.current) return; state.confirmedSpeed.current = target; } catch (cause) { + if (!state.active.current) return; if (state.desiredSpeed.current === target) { state.desiredSpeed.current = state.confirmedSpeed.current; callbacks.setSpeed(state.confirmedSpeed.current);