From 89bf7e23647ba170a0530137758fba6bd0f46f02 Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Tue, 12 May 2026 14:01:36 +0200 Subject: [PATCH] =?UTF-8?q?refactor(player):=20E.9=20=E2=80=94=20extract?= =?UTF-8?q?=20scheduled=20pause/resume=20timer=20lifecycle=20(#572)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two timer mutables (`scheduledPauseTimer`, `scheduledResumeTimer`) plus the three clear helpers move into `src/store/scheduleTimers.ts`. The module also gains `schedulePauseTimer(delayMs, onFire)` / `scheduleResumeTimer(delayMs, onFire)` so callers no longer need to do the `window.setTimeout(...) as unknown as number` cast or null the handle inside the fire callback — the module auto-clears its own reference before invoking the user callback. `schedulePauseIn` / `scheduleResumeIn` store actions are now four lines shorter each and don't reach into the timer mutables. playerStore 3389 → 3369 LOC. 9 focused tests cover schedule + fire + clear + replace-on-reschedule + independence between the two timers. --- src/store/playerStore.ts | 42 ++++----------- src/store/scheduleTimers.test.ts | 93 ++++++++++++++++++++++++++++++++ src/store/scheduleTimers.ts | 50 +++++++++++++++++ 3 files changed, 154 insertions(+), 31 deletions(-) create mode 100644 src/store/scheduleTimers.test.ts create mode 100644 src/store/scheduleTimers.ts diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index ac205163..a02c3b67 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -58,6 +58,13 @@ import { markLoudnessStable, setCachedLoudnessGain, } from './loudnessGainCache'; +import { + clearAllPlaybackScheduleTimers, + clearScheduledPauseTimers, + clearScheduledResumeTimers, + schedulePauseTimer, + scheduleResumeTimer, +} from './scheduleTimers'; // Re-export the playback-progress public surface so existing call sites // (PlayerBar, FullscreenPlayer, WaveformSeek, LyricsPane, MobilePlayerView, @@ -451,29 +458,6 @@ const STORE_PROGRESS_COMMIT_MIN_DELTA_SEC = 5.0; let lastStoreProgressCommitAt = 0; -/** Deferred pause / resume — cleared on stop, new track, manual pause/resume. */ -let scheduledPauseTimer: number | null = null; -let scheduledResumeTimer: number | null = null; - -function clearScheduledPauseTimers() { - if (scheduledPauseTimer != null) { - window.clearTimeout(scheduledPauseTimer); - scheduledPauseTimer = null; - } -} - -function clearScheduledResumeTimers() { - if (scheduledResumeTimer != null) { - window.clearTimeout(scheduledResumeTimer); - scheduledResumeTimer = null; - } -} - -function clearAllPlaybackScheduleTimers() { - clearScheduledPauseTimers(); - clearScheduledResumeTimers(); -} - function setSeekTarget(seconds: number) { seekTarget = seconds; seekTargetSetAt = Date.now(); @@ -2686,32 +2670,28 @@ export const usePlayerStore = create()( schedulePauseIn: (seconds) => { const s = get(); if (!s.isPlaying) return; - clearScheduledPauseTimers(); const delayMs = Math.max(500, Math.round(Number(seconds) * 1000)); const startedAt = Date.now(); const at = startedAt + delayMs; set({ scheduledPauseAtMs: at, scheduledPauseStartMs: startedAt }); - scheduledPauseTimer = window.setTimeout(() => { - scheduledPauseTimer = null; + schedulePauseTimer(delayMs, () => { set({ scheduledPauseAtMs: null, scheduledPauseStartMs: null }); get().pause(); - }, delayMs) as unknown as number; + }); }, scheduleResumeIn: (seconds) => { const s = get(); if (s.isPlaying) return; if (!s.currentTrack && !s.currentRadio) return; - clearScheduledResumeTimers(); const delayMs = Math.max(500, Math.round(Number(seconds) * 1000)); const startedAt = Date.now(); const at = startedAt + delayMs; set({ scheduledResumeAtMs: at, scheduledResumeStartMs: startedAt }); - scheduledResumeTimer = window.setTimeout(() => { - scheduledResumeTimer = null; + scheduleResumeTimer(delayMs, () => { set({ scheduledResumeAtMs: null, scheduledResumeStartMs: null }); get().resume(); - }, delayMs) as unknown as number; + }); }, togglePlay: () => { diff --git a/src/store/scheduleTimers.test.ts b/src/store/scheduleTimers.test.ts new file mode 100644 index 00000000..e3062c17 --- /dev/null +++ b/src/store/scheduleTimers.test.ts @@ -0,0 +1,93 @@ +/** + * Deferred pause/resume timer lifecycle. Vitest fake timers drive the + * setTimeout/clearTimeout pair; the new schedule helpers fire the callback + * exactly once, auto-clearing their internal handle so a follow-up + * `clearAll…` is idempotent. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + _resetScheduleTimersForTest, + clearAllPlaybackScheduleTimers, + clearScheduledPauseTimers, + clearScheduledResumeTimers, + schedulePauseTimer, + scheduleResumeTimer, +} from './scheduleTimers'; + +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + _resetScheduleTimersForTest(); + vi.useRealTimers(); +}); + +describe('schedulePauseTimer', () => { + it('fires the callback exactly once after the delay', () => { + const cb = vi.fn(); + schedulePauseTimer(1000, cb); + vi.advanceTimersByTime(999); + expect(cb).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('replaces an outstanding timer when called again (only the latest fires)', () => { + const first = vi.fn(); + const second = vi.fn(); + schedulePauseTimer(500, first); + schedulePauseTimer(500, second); + vi.advanceTimersByTime(500); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + }); + + it('clearScheduledPauseTimers cancels a pending fire', () => { + const cb = vi.fn(); + schedulePauseTimer(500, cb); + clearScheduledPauseTimers(); + vi.advanceTimersByTime(1000); + expect(cb).not.toHaveBeenCalled(); + }); + + it('clearScheduledPauseTimers is a no-op when nothing is pending', () => { + expect(() => clearScheduledPauseTimers()).not.toThrow(); + }); +}); + +describe('scheduleResumeTimer', () => { + it('fires after the delay and clears its handle', () => { + const cb = vi.fn(); + scheduleResumeTimer(750, cb); + vi.advanceTimersByTime(750); + expect(cb).toHaveBeenCalledTimes(1); + // Subsequent clear is a no-op (timer already cleared itself on fire). + expect(() => clearScheduledResumeTimers()).not.toThrow(); + }); + + it('runs independently from the pause timer', () => { + const pause = vi.fn(); + const resume = vi.fn(); + schedulePauseTimer(500, pause); + scheduleResumeTimer(800, resume); + vi.advanceTimersByTime(500); + expect(pause).toHaveBeenCalledTimes(1); + expect(resume).not.toHaveBeenCalled(); + vi.advanceTimersByTime(300); + expect(resume).toHaveBeenCalledTimes(1); + }); +}); + +describe('clearAllPlaybackScheduleTimers', () => { + it('cancels both pending timers in one call', () => { + const pause = vi.fn(); + const resume = vi.fn(); + schedulePauseTimer(500, pause); + scheduleResumeTimer(500, resume); + clearAllPlaybackScheduleTimers(); + vi.advanceTimersByTime(1000); + expect(pause).not.toHaveBeenCalled(); + expect(resume).not.toHaveBeenCalled(); + }); +}); diff --git a/src/store/scheduleTimers.ts b/src/store/scheduleTimers.ts new file mode 100644 index 00000000..838c1c38 --- /dev/null +++ b/src/store/scheduleTimers.ts @@ -0,0 +1,50 @@ +/** + * Deferred pause / resume timers — back the `schedulePauseIn` / + * `scheduleResumeIn` store actions. Encapsulated so the timer handles + * never leak: every public API either schedules + auto-clears on fire, + * or clears an outstanding timer outright. Cleared on stop, new track, + * manual pause/resume. + */ + +let scheduledPauseTimer: number | null = null; +let scheduledResumeTimer: number | null = null; + +export function schedulePauseTimer(delayMs: number, onFire: () => void): void { + clearScheduledPauseTimers(); + scheduledPauseTimer = window.setTimeout(() => { + scheduledPauseTimer = null; + onFire(); + }, delayMs) as unknown as number; +} + +export function scheduleResumeTimer(delayMs: number, onFire: () => void): void { + clearScheduledResumeTimers(); + scheduledResumeTimer = window.setTimeout(() => { + scheduledResumeTimer = null; + onFire(); + }, delayMs) as unknown as number; +} + +export function clearScheduledPauseTimers(): void { + if (scheduledPauseTimer != null) { + window.clearTimeout(scheduledPauseTimer); + scheduledPauseTimer = null; + } +} + +export function clearScheduledResumeTimers(): void { + if (scheduledResumeTimer != null) { + window.clearTimeout(scheduledResumeTimer); + scheduledResumeTimer = null; + } +} + +export function clearAllPlaybackScheduleTimers(): void { + clearScheduledPauseTimers(); + clearScheduledResumeTimers(); +} + +/** Test-only: clear both handles without invoking the timer callbacks. */ +export function _resetScheduleTimersForTest(): void { + clearAllPlaybackScheduleTimers(); +}