From 472b9d9e84bcc01228ddf46e0bf12302112a4d52 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 3 May 2026 21:37:55 -0400 Subject: [PATCH] fix(web/m7-364): untrack _position in queue immediate-write effect Wrap the _position read inside untrack() in the immediate-write $effect so it no longer registers as a reactive dependency, preventing the effect from firing on every 4Hz position tick. Also adds three missing restoreQueue unit tests with vi.mock for the auth store. Co-Authored-By: Claude Sonnet 4.6 --- web/src/lib/player/store.svelte.ts | 18 +++++++----- web/src/lib/player/store.test.ts | 47 +++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index 87d97732..be683a72 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -1,4 +1,5 @@ import type { TrackRef, RadioResponse } from '$lib/api/types'; +import { untrack } from 'svelte'; import { api } from '$lib/api/client'; import { user } from '$lib/auth/store.svelte'; import { readPersistedQueue, writePersistedQueue } from './persisted'; @@ -379,20 +380,21 @@ $effect.root(() => { $effect.root(() => { let lastPositionWriteAt = 0; - // Immediate write on queue or index changes. + // Immediate write on queue or index changes. Position is read via + // untrack so this effect doesn't fire on every position tick — only + // queue/index changes drive it. The throttled-write effect below + // covers position drift. $effect(() => { const userId = user.value?.id; if (!userId) return; - // Read these to register the dependency. Position is also read but - // its standalone changes are handled by the second effect below. - const _q = _queue; - const _i = _index; - void _q; - void _i; + // Reading these registers the reactive dependency; the values + // themselves are unused — writePersistedQueue re-reads live state. + _queue; + _index; writePersistedQueue(userId, { queue: _queue, index: _index, - position: _position + position: untrack(() => _position) }); lastPositionWriteAt = Date.now(); }); diff --git a/web/src/lib/player/store.test.ts b/web/src/lib/player/store.test.ts index e621953b..2d894db7 100644 --- a/web/src/lib/player/store.test.ts +++ b/web/src/lib/player/store.test.ts @@ -14,9 +14,19 @@ import { toggleShuffle, cycleRepeat, reportStateFromAudio, - __setShuffleRng + __setShuffleRng, + restoreQueue } from './store.svelte'; import type { TrackRef } from '$lib/api/types'; +import { writePersistedQueue, clearPersistedQueue } from './persisted'; + +vi.mock('$lib/auth/store.svelte', () => ({ + user: { + get value() { + return { id: 'test-user' }; + } + } +})); function track(id: string, dur = 180): TrackRef { return { @@ -546,4 +556,39 @@ describe('queue mutations', () => { closeQueueDrawer(); expect(player.queueDrawerOpen).toBe(false); }); + + test('restoreQueue rehydrates queue, index, position and parks state at paused', () => { + // Seed localStorage manually so we don't depend on the persistence + // effect (which is integration-tested at the layout level). + const userId = 'test-user'; + writePersistedQueue(userId, { + queue: [track('a'), track('b'), track('c')], + index: 1, + position: 42.5 + }); + restoreQueue(userId); + expect(player.queue.map((x) => x.id)).toEqual(['a', 'b', 'c']); + expect(player.index).toBe(1); + expect(player.position).toBe(42.5); + expect(player.state).toBe('paused'); + clearPersistedQueue(userId); + }); + + test('restoreQueue clamps out-of-range index', () => { + const userId = 'test-user'; + writePersistedQueue(userId, { + queue: [track('a'), track('b'), track('c')], + index: 99, + position: 0 + }); + restoreQueue(userId); + expect(player.index).toBe(2); + clearPersistedQueue(userId); + }); + + test('restoreQueue with empty userId is a no-op', () => { + playQueue([track('a')], 0); + restoreQueue(''); + expect(player.queue.map((x) => x.id)).toEqual(['a']); + }); });