refactor(player): E.2 — extract queue-undo machinery into queueUndo.ts (#565)

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.
2026-07-21 23:05:46 +00:00 · 2026-05-12 11:25:26 +02:00
parent dcf3dd98e0
commit 1521e4ea8f
3 changed files with 249 additions and 70 deletions
@@ -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<string, number> = {};
 let stableLoudnessGainByTrackId: Record<string, true> = {};
 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<PlayerState>()(
          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<PlayerState>()(
          });
        }
        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<PlayerState>()(

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

@@ -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> = {}): 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();
+  });
+});
@@ -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();
+}