From 1521e4ea8feb2348c24ef78a7a37bee61adfcb6f Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Tue, 12 May 2026 11:25:26 +0200 Subject: [PATCH] =?UTF-8?q?refactor(player):=20E.2=20=E2=80=94=20extract?= =?UTF-8?q?=20queue-undo=20machinery=20into=20queueUndo.ts=20(#565)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the bounded undo/redo stacks, snapshot factory, scroll-top reader registry, and pending-scroll-top channel into a dedicated module under src/store/. playerStore re-exports the three public APIs that QueuePanel and the test harness already imported (registerQueueListScrollTopReader, consumePendingQueueListScrollTop, _resetQueueUndoStacksForTest), so no caller-side changes are needed. The new module adds explicit push/pop accessors so the undo/redo store actions stop reaching into module-scoped arrays directly. Inline `pendingQueueListScrollTop = …` writes inside applyQueueHistorySnapshot become `setPendingQueueListScrollTop(…)` calls — same effect, scoped through the module. PlayerState gets the `export` keyword so queueUndo.ts can type its state-shaped parameters. playerStore.ts 3618 → 3576 LOC. queueUndo gains a focused test file covering snapshot deep-cloning, the redo-stack invalidation on a fresh undo push, max-size enforcement, and the scroll-top channel. Pre-PR check: PASS. --- src/store/playerStore.ts | 98 ++++++++------------------- src/store/queueUndo.test.ts | 128 ++++++++++++++++++++++++++++++++++++ src/store/queueUndo.ts | 93 ++++++++++++++++++++++++++ 3 files changed, 249 insertions(+), 70 deletions(-) create mode 100644 src/store/queueUndo.test.ts create mode 100644 src/store/queueUndo.ts diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index 3c24cf76..21b85b8c 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -31,12 +31,33 @@ import { shallowCloneQueueTracks, } from '../utils/queueIdentity'; import { getWindowKind } from '../app/windowKind'; +import { + _resetQueueUndoStacksForTest, + consumePendingQueueListScrollTop, + popQueueRedoSnapshot, + popQueueUndoSnapshot, + pushQueueRedoSnapshot, + pushQueueUndoFromGetter, + pushQueueUndoSnapshot, + queueUndoSnapshotFromState, + registerQueueListScrollTopReader, + setPendingQueueListScrollTop, + type QueueUndoSnapshot, +} from './queueUndo'; // Re-export for backward compatibility with the ~30 call sites that still // import these helpers from playerStore. Phase E (store splits) will migrate // the imports to '../utils/*' directly and drop these re-exports. export { resolveReplayGainDb, shuffleArray, songToTrack }; +// Re-export the queue-undo public API so existing callers (QueuePanel, +// test/helpers/storeReset) keep their `from './playerStore'` imports. +export { + _resetQueueUndoStacksForTest, + consumePendingQueueListScrollTop, + registerQueueListScrollTopReader, +}; + const QUEUE_VISIBILITY_STORAGE_KEY = 'psysonic_queue_visible'; function readInitialQueueVisibility(): boolean { @@ -91,7 +112,7 @@ export interface Track { playNextAdded?: boolean; } -interface PlayerState { +export interface PlayerState { currentTrack: Track | null; waveformBins: number[] | null; normalizationNowDb: number | null; @@ -334,67 +355,6 @@ let cachedLoudnessGainByTrackId: Record = {}; let stableLoudnessGainByTrackId: Record = {}; let lastNormalizationUiUpdateAtMs = 0; -/** Bounded stack of queue snapshots for Ctrl+Z / Cmd+Z undo. */ -const QUEUE_UNDO_MAX = 32; -type QueueUndoSnapshot = { - queue: Track[]; - queueIndex: number; - currentTrack: Track | null; - /** Seconds — captured with the snapshot (older entries may omit). */ - currentTime?: number; - progress?: number; - isPlaying?: boolean; - /** Main queue panel list `scrollTop` when the snapshot was taken. */ - queueListScrollTop?: number; -}; -const queueUndoStack: QueueUndoSnapshot[] = []; -const queueRedoStack: QueueUndoSnapshot[] = []; - -/** Test-only: clears module-scoped undo/redo stacks so each test starts clean. */ -export function _resetQueueUndoStacksForTest(): void { - queueUndoStack.length = 0; - queueRedoStack.length = 0; -} - -/** QueuePanel registers a reader so undo snapshots capture list scroll position. */ -let queueListScrollTopReader: (() => number | undefined) | null = null; - -export function registerQueueListScrollTopReader(reader: (() => number | undefined) | null): void { - queueListScrollTopReader = reader; -} - -function readQueueListScrollTopForUndo(): number | undefined { - return queueListScrollTopReader?.() ?? undefined; -} - -/** Set in applyQueueHistorySnapshot; QueuePanel consumes in useLayoutEffect after commit. */ -let pendingQueueListScrollTop: number | undefined; - -export function consumePendingQueueListScrollTop(): number | undefined { - const v = pendingQueueListScrollTop; - pendingQueueListScrollTop = undefined; - return v; -} - -function queueUndoSnapshotFromState(s: PlayerState): QueueUndoSnapshot { - const scrollTop = readQueueListScrollTopForUndo(); - return { - queue: shallowCloneQueueTracks(s.queue), - queueIndex: s.queueIndex, - currentTrack: s.currentTrack ? { ...s.currentTrack } : null, - currentTime: s.currentTime, - progress: s.progress, - isPlaying: s.isPlaying, - ...(scrollTop !== undefined ? { queueListScrollTop: scrollTop } : {}), - }; -} - -function pushQueueUndoFromGetter(get: () => PlayerState) { - queueRedoStack.length = 0; - queueUndoStack.push(queueUndoSnapshotFromState(get())); - while (queueUndoStack.length > QUEUE_UNDO_MAX) queueUndoStack.shift(); -} - /** Reload Rust audio to match a queue-undo snapshot (Zustand alone does not move the engine). */ function queueUndoRestoreAudioEngine(opts: { generation: number; @@ -2142,7 +2102,7 @@ export const usePlayerStore = create()( isAudioPaused = false; syncQueueToServer(nextQueue, null, 0); if (typeof snap.queueListScrollTop === 'number' && Number.isFinite(snap.queueListScrollTop)) { - pendingQueueListScrollTop = Math.max(0, snap.queueListScrollTop); + setPendingQueueListScrollTop(Math.max(0, snap.queueListScrollTop)); } return true; } @@ -2165,7 +2125,7 @@ export const usePlayerStore = create()( }); } if (typeof snap.queueListScrollTop === 'number' && Number.isFinite(snap.queueListScrollTop)) { - pendingQueueListScrollTop = Math.max(0, snap.queueListScrollTop); + setPendingQueueListScrollTop(Math.max(0, snap.queueListScrollTop)); } syncQueueToServer(nextQueue, nextTrack, tRestore); return true; @@ -3396,19 +3356,17 @@ export const usePlayerStore = create()( undoLastQueueEdit: () => { const prior = get(); - const snap = queueUndoStack.pop(); + const snap = popQueueUndoSnapshot(); if (!snap) return false; - queueRedoStack.push(queueUndoSnapshotFromState(prior)); - while (queueRedoStack.length > QUEUE_UNDO_MAX) queueRedoStack.shift(); + pushQueueRedoSnapshot(queueUndoSnapshotFromState(prior)); return applyQueueHistorySnapshot(snap, prior); }, redoLastQueueEdit: () => { const prior = get(); - const snap = queueRedoStack.pop(); + const snap = popQueueRedoSnapshot(); if (!snap) return false; - queueUndoStack.push(queueUndoSnapshotFromState(prior)); - while (queueUndoStack.length > QUEUE_UNDO_MAX) queueUndoStack.shift(); + pushQueueUndoSnapshot(queueUndoSnapshotFromState(prior)); return applyQueueHistorySnapshot(snap, prior); }, diff --git a/src/store/queueUndo.test.ts b/src/store/queueUndo.test.ts new file mode 100644 index 00000000..f01c83f8 --- /dev/null +++ b/src/store/queueUndo.test.ts @@ -0,0 +1,128 @@ +/** + * Module-scoped queue undo / redo stack. The interesting behaviours are + * (a) max-size enforcement, (b) the redo stack being wiped on a fresh undo + * push, and (c) the scroll-top reader / consumer pair that QueuePanel uses + * to restore list scroll position after an undo/redo commit. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import type { PlayerState, Track } from './playerStore'; +import { + QUEUE_UNDO_MAX, + _resetQueueUndoStacksForTest, + consumePendingQueueListScrollTop, + popQueueRedoSnapshot, + popQueueUndoSnapshot, + pushQueueRedoSnapshot, + pushQueueUndoFromGetter, + pushQueueUndoSnapshot, + queueUndoSnapshotFromState, + registerQueueListScrollTopReader, + setPendingQueueListScrollTop, +} from './queueUndo'; + +function track(id: string): Track { + return { id, title: id, artist: 'A', album: 'X', albumId: 'X', duration: 100 }; +} + +function state(queue: Track[], overrides: Partial = {}): PlayerState { + return { + queue, + queueIndex: 0, + currentTrack: queue[0] ?? null, + currentTime: 0, + progress: 0, + isPlaying: false, + ...overrides, + } as PlayerState; +} + +beforeEach(() => { + _resetQueueUndoStacksForTest(); + registerQueueListScrollTopReader(null); + // Drain any leftover pending scroll-top + consumePendingQueueListScrollTop(); +}); + +describe('queueUndoSnapshotFromState', () => { + it('deep-clones queue tracks and currentTrack', () => { + const original = state([track('a'), track('b')]); + const snap = queueUndoSnapshotFromState(original); + expect(snap.queue).not.toBe(original.queue); + expect(snap.queue[0]).not.toBe(original.queue[0]); + expect(snap.currentTrack).not.toBe(original.currentTrack); + expect(snap.queue.map(t => t.id)).toEqual(['a', 'b']); + }); + + it('preserves currentTrack=null', () => { + const snap = queueUndoSnapshotFromState(state([])); + expect(snap.currentTrack).toBeNull(); + }); + + it('includes queueListScrollTop only when a reader is registered', () => { + expect(queueUndoSnapshotFromState(state([track('a')])).queueListScrollTop).toBeUndefined(); + registerQueueListScrollTopReader(() => 240); + expect(queueUndoSnapshotFromState(state([track('a')])).queueListScrollTop).toBe(240); + }); +}); + +describe('pushQueueUndoFromGetter', () => { + it('captures the current state on top of the undo stack', () => { + pushQueueUndoFromGetter(() => state([track('a')])); + const snap = popQueueUndoSnapshot(); + expect(snap?.queue[0].id).toBe('a'); + }); + + it('wipes the redo stack — a fresh action invalidates redo history', () => { + pushQueueRedoSnapshot(queueUndoSnapshotFromState(state([track('z')]))); + pushQueueUndoFromGetter(() => state([track('a')])); + expect(popQueueRedoSnapshot()).toBeUndefined(); + }); + + it(`caps the undo stack at QUEUE_UNDO_MAX (=${QUEUE_UNDO_MAX})`, () => { + for (let i = 0; i < QUEUE_UNDO_MAX + 5; i++) { + pushQueueUndoFromGetter(() => state([track(`t${i}`)])); + } + let depth = 0; + while (popQueueUndoSnapshot()) depth++; + expect(depth).toBe(QUEUE_UNDO_MAX); + }); +}); + +describe('pushQueueUndoSnapshot / pushQueueRedoSnapshot', () => { + it('respect QUEUE_UNDO_MAX when pushing prebuilt snapshots', () => { + const snap = queueUndoSnapshotFromState(state([track('a')])); + for (let i = 0; i < QUEUE_UNDO_MAX + 3; i++) pushQueueRedoSnapshot(snap); + let depth = 0; + while (popQueueRedoSnapshot()) depth++; + expect(depth).toBe(QUEUE_UNDO_MAX); + }); + + it('undo-snapshot push keeps order LIFO', () => { + pushQueueUndoSnapshot(queueUndoSnapshotFromState(state([track('first')]))); + pushQueueUndoSnapshot(queueUndoSnapshotFromState(state([track('second')]))); + expect(popQueueUndoSnapshot()?.queue[0].id).toBe('second'); + expect(popQueueUndoSnapshot()?.queue[0].id).toBe('first'); + }); +}); + +describe('_resetQueueUndoStacksForTest', () => { + it('clears both stacks', () => { + pushQueueUndoFromGetter(() => state([track('a')])); + pushQueueRedoSnapshot(queueUndoSnapshotFromState(state([track('b')]))); + _resetQueueUndoStacksForTest(); + expect(popQueueUndoSnapshot()).toBeUndefined(); + expect(popQueueRedoSnapshot()).toBeUndefined(); + }); +}); + +describe('pending queue-list scroll-top', () => { + it('returns undefined when nothing was set', () => { + expect(consumePendingQueueListScrollTop()).toBeUndefined(); + }); + + it('round-trips a stored value once and then drains', () => { + setPendingQueueListScrollTop(512); + expect(consumePendingQueueListScrollTop()).toBe(512); + expect(consumePendingQueueListScrollTop()).toBeUndefined(); + }); +}); diff --git a/src/store/queueUndo.ts b/src/store/queueUndo.ts new file mode 100644 index 00000000..a6b72be1 --- /dev/null +++ b/src/store/queueUndo.ts @@ -0,0 +1,93 @@ +import type { PlayerState, Track } from './playerStore'; + +/** Hard cap on undo/redo depth — keeps memory bounded for very long sessions. */ +export const QUEUE_UNDO_MAX = 32; + +export type QueueUndoSnapshot = { + queue: Track[]; + queueIndex: number; + currentTrack: Track | null; + /** Seconds — captured with the snapshot (older entries may omit). */ + currentTime?: number; + progress?: number; + isPlaying?: boolean; + /** Main queue panel list `scrollTop` when the snapshot was taken. */ + queueListScrollTop?: number; +}; + +const queueUndoStack: QueueUndoSnapshot[] = []; +const queueRedoStack: QueueUndoSnapshot[] = []; + +/** Test-only: clears module-scoped undo/redo stacks so each test starts clean. */ +export function _resetQueueUndoStacksForTest(): void { + queueUndoStack.length = 0; + queueRedoStack.length = 0; +} + +/** QueuePanel registers a reader so undo snapshots capture list scroll position. */ +let queueListScrollTopReader: (() => number | undefined) | null = null; + +export function registerQueueListScrollTopReader(reader: (() => number | undefined) | null): void { + queueListScrollTopReader = reader; +} + +function readQueueListScrollTopForUndo(): number | undefined { + return queueListScrollTopReader?.() ?? undefined; +} + +/** Set in applyQueueHistorySnapshot; QueuePanel consumes in useLayoutEffect after commit. */ +let pendingQueueListScrollTop: number | undefined; + +export function setPendingQueueListScrollTop(value: number): void { + pendingQueueListScrollTop = value; +} + +export function consumePendingQueueListScrollTop(): number | undefined { + const v = pendingQueueListScrollTop; + pendingQueueListScrollTop = undefined; + return v; +} + +export function queueUndoSnapshotFromState(s: PlayerState): QueueUndoSnapshot { + const scrollTop = readQueueListScrollTopForUndo(); + return { + queue: s.queue.map(t => ({ ...t })), + queueIndex: s.queueIndex, + currentTrack: s.currentTrack ? { ...s.currentTrack } : null, + currentTime: s.currentTime, + progress: s.progress, + isPlaying: s.isPlaying, + ...(scrollTop !== undefined ? { queueListScrollTop: scrollTop } : {}), + }; +} + +/** + * Snapshot the current state onto the undo stack and clear the redo stack — + * standard "new action invalidates redo history" behaviour. Called from every + * mutating queue action. + */ +export function pushQueueUndoFromGetter(get: () => PlayerState): void { + queueRedoStack.length = 0; + queueUndoStack.push(queueUndoSnapshotFromState(get())); + while (queueUndoStack.length > QUEUE_UNDO_MAX) queueUndoStack.shift(); +} + +export function popQueueUndoSnapshot(): QueueUndoSnapshot | undefined { + return queueUndoStack.pop(); +} + +export function popQueueRedoSnapshot(): QueueUndoSnapshot | undefined { + return queueRedoStack.pop(); +} + +/** Push a prebuilt snapshot onto the redo stack — used after an undo step. */ +export function pushQueueRedoSnapshot(snap: QueueUndoSnapshot): void { + queueRedoStack.push(snap); + while (queueRedoStack.length > QUEUE_UNDO_MAX) queueRedoStack.shift(); +} + +/** Push a prebuilt snapshot onto the undo stack — used after a redo step. */ +export function pushQueueUndoSnapshot(snap: QueueUndoSnapshot): void { + queueUndoStack.push(snap); + while (queueUndoStack.length > QUEUE_UNDO_MAX) queueUndoStack.shift(); +}