Cancel stale playback speed saves

Signed-off-by: John Tennant <jtennant@block.xyz>

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<string | null>(null);
   const [loaded, setLoaded] = useState(false);
+  const active = useRef(true);
   const confirmedSpeed = useRef(DEFAULT_SPEED);
   const desiredSpeed = useRef(DEFAULT_SPEED);
   const flushPromise = useRef<Promise<void> | 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,
     );

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);
+});

desktop/src/features/settings/ui/playbackSpeedPersistence.ts

@@ -3,6 +3,7 @@ type MutableValue<T> = {
 };
 
 export type PlaybackSpeedPersistenceState = {
+  active: MutableValue<boolean>;
   confirmedSpeed: MutableValue<number>;
   desiredSpeed: MutableValue<number>;
   flushPromise: MutableValue<Promise<void> | 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<PlaybackSpeedPersistenceCallbacks, "setSpeed">,
@@ -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);