From d2f2085f1f9edc6fcfb9621901cd63ac86d579f6 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:24:11 +0200 Subject: [PATCH] test(e4): readSelectedId ?pl guard, useUrlMirror dep-keying, usePref flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining E4 deep-review deferred gaps, in one sprint: - readSelectedId split into playlistsInit.ts (react-refresh-clean, testable) — ?pl accepted only as a positive integer; empty/0/negative/fractional/non-numeric fall back to the stored selection; null when neither is present. - useUrlMirror: mounts write the param, a same-serialized rerender does NOT re-fire (stable dep), a null value clears the key, other params preserved. - usePref: a pending debounced write is FLUSHED on unmount (not dropped); an unmount with nothing pending stays silent. 20 new test cases; full FE gate green (tsc/eslint/prettier/knip, 309 vitest). --- frontend/src/components/PlaylistsProvider.tsx | 14 +---- frontend/src/components/playlistsInit.test.ts | 54 +++++++++++++++++++ frontend/src/components/playlistsInit.ts | 14 +++++ frontend/src/lib/usePref.dom.test.ts | 23 ++++++++ frontend/src/lib/useUrlMirror.dom.test.ts | 51 ++++++++++++++++++ 5 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 frontend/src/components/playlistsInit.test.ts create mode 100644 frontend/src/components/playlistsInit.ts create mode 100644 frontend/src/lib/useUrlMirror.dom.test.ts diff --git a/frontend/src/components/PlaylistsProvider.tsx b/frontend/src/components/PlaylistsProvider.tsx index e837da9c..4ddf94fe 100644 --- a/frontend/src/components/PlaylistsProvider.tsx +++ b/frontend/src/components/PlaylistsProvider.tsx @@ -1,8 +1,9 @@ import { createContext, useCallback, useContext, useMemo, useState, type ReactNode } from "react"; -import { getAccountRaw, LS, setAccountRaw } from "../lib/storage"; +import { LS, setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { useUrlMirror } from "../lib/useUrlMirror"; import { useNavigation } from "./NavigationProvider"; +import { readSelectedId } from "./playlistsInit"; // Owns the Playlists module's shared local state — the rail's name filter and the selected playlist // — lifted out of the App god component so the header search box, the PlaylistsRail, and the @@ -38,17 +39,6 @@ export function usePlaylistsActions(): PlaylistsActions { return useContext(ActionsContext); } -function readSelectedId(): number | null { - // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a - // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id, - // both of which would select a non-existent playlist. - const fromUrl = new URLSearchParams(window.location.search).get("pl"); - const n = fromUrl ? Number(fromUrl) : NaN; - if (Number.isInteger(n) && n > 0) return n; - const s = getAccountRaw(LS.playlist); - return s ? Number(s) : null; -} - export function PlaylistsProvider({ children }: { children: ReactNode }) { const [search, setSearchState] = useState(""); const [selectedId, setSelectedIdState] = useState(readSelectedId); diff --git a/frontend/src/components/playlistsInit.test.ts b/frontend/src/components/playlistsInit.test.ts new file mode 100644 index 00000000..bdaf7aaa --- /dev/null +++ b/frontend/src/components/playlistsInit.test.ts @@ -0,0 +1,54 @@ +// @vitest-environment jsdom +// +// readSelectedId: the Playlists selected-id seed. Backfills the E4 deep-review's deferred ?pl guard +// coverage — a ?pl bookmark wins over the stored selection but only when it's a positive integer; +// anything else (empty / zero / negative / fractional / non-numeric) falls back to the stored id. +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const { stored } = vi.hoisted(() => ({ stored: { v: null as string | null } })); +vi.mock("../lib/storage", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAccountRaw: () => stored.v }; +}); + +import { readSelectedId } from "./playlistsInit"; + +const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); + +beforeEach(() => { + setUrl("/"); + stored.v = null; +}); + +describe("readSelectedId ?pl guard", () => { + it("accepts a positive-integer ?pl", () => { + setUrl("?pl=5"); + expect(readSelectedId()).toBe(5); + }); + + it("a ?pl bookmark wins over the stored selection", () => { + setUrl("?pl=5"); + stored.v = "7"; + expect(readSelectedId()).toBe(5); + }); + + it.each(["", "0", "-2", "1.5", "abc"])("rejects ?pl=%s and falls back to stored", (raw) => { + setUrl(`?pl=${raw}`); + stored.v = "7"; + expect(readSelectedId()).toBe(7); + }); + + it("rejects a bad ?pl and returns null when there is no stored selection", () => { + setUrl("?pl=0"); + expect(readSelectedId()).toBeNull(); + }); + + it("uses the stored selection when there is no ?pl", () => { + stored.v = "9"; + expect(readSelectedId()).toBe(9); + }); + + it("returns null when neither ?pl nor a stored selection is present", () => { + expect(readSelectedId()).toBeNull(); + }); +}); diff --git a/frontend/src/components/playlistsInit.ts b/frontend/src/components/playlistsInit.ts new file mode 100644 index 00000000..bc3c256b --- /dev/null +++ b/frontend/src/components/playlistsInit.ts @@ -0,0 +1,14 @@ +import { getAccountRaw, LS } from "../lib/storage"; + +// The Playlists module's selected-id seed, split out of PlaylistsProvider so its ?pl guard is +// unit-testable and the provider stays react-refresh-clean (component-only exports). +export function readSelectedId(): number | null { + // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a + // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id, + // both of which would select a non-existent playlist. + const fromUrl = new URLSearchParams(window.location.search).get("pl"); + const n = fromUrl ? Number(fromUrl) : NaN; + if (Number.isInteger(n) && n > 0) return n; + const s = getAccountRaw(LS.playlist); + return s ? Number(s) : null; +} diff --git a/frontend/src/lib/usePref.dom.test.ts b/frontend/src/lib/usePref.dom.test.ts index e20831da..a9fb61aa 100644 --- a/frontend/src/lib/usePref.dom.test.ts +++ b/frontend/src/lib/usePref.dom.test.ts @@ -155,6 +155,29 @@ describe("usePref", () => { expect(savePrefs).toHaveBeenCalledTimes(1); expect(savePrefs).toHaveBeenCalledWith({ vol: 30 }); }); + + it("flushes a pending debounced write on unmount instead of dropping it", () => { + const qc = makeClient(); + seedMe(qc, {}); + const { result, unmount } = renderHook(() => usePref("vol", 0, { debounceMs: 300 }), { + wrapper: wrapperFor(qc), + }); + act(() => result.current[1](42)); + expect(savePrefs).not.toHaveBeenCalled(); // still inside the debounce window + unmount(); // cleanup must flush the pending write, not lose it + expect(savePrefs).toHaveBeenCalledTimes(1); + expect(savePrefs).toHaveBeenCalledWith({ vol: 42 }); + }); + + it("unmount with no pending write does not call savePrefs", () => { + const qc = makeClient(); + seedMe(qc, {}); + const { unmount } = renderHook(() => usePref("vol", 0, { debounceMs: 300 }), { + wrapper: wrapperFor(qc), + }); + unmount(); + expect(savePrefs).not.toHaveBeenCalled(); + }); }); }); diff --git a/frontend/src/lib/useUrlMirror.dom.test.ts b/frontend/src/lib/useUrlMirror.dom.test.ts new file mode 100644 index 00000000..d1d85f55 --- /dev/null +++ b/frontend/src/lib/useUrlMirror.dom.test.ts @@ -0,0 +1,51 @@ +// @vitest-environment jsdom +// +// useUrlMirror: mirror a set of nav params onto the CURRENT history entry (replaceState) whenever they +// change. Backfills the E4 deep-review's deferred coverage — the effect keys on the SERIALIZED updates +// so an inline object literal (new identity every render) only fires when a param actually changes; a +// null value clears a key; and every other param on the URL is preserved. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { renderHook } from "@testing-library/react"; +import { useUrlMirror } from "./useUrlMirror"; + +const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); +let spy: ReturnType; + +beforeEach(() => { + setUrl("/"); + spy = vi.spyOn(window.history, "replaceState"); +}); +afterEach(() => spy.mockRestore()); + +describe("useUrlMirror", () => { + it("writes the param onto the current entry on mount", () => { + renderHook(() => useUrlMirror({ pl: "5" })); + expect(window.location.search).toBe("?pl=5"); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("does not re-fire when the serialized updates are unchanged (stable dep)", () => { + const { rerender } = renderHook((p: Record) => useUrlMirror(p), { + initialProps: { pl: "5" } as Record, + }); + expect(spy).toHaveBeenCalledTimes(1); + rerender({ pl: "5" }); // new object identity, same serialized value + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("clears a key when its value turns null", () => { + const { rerender } = renderHook((p: Record) => useUrlMirror(p), { + initialProps: { pl: "5" } as Record, + }); + expect(window.location.search).toBe("?pl=5"); + rerender({ pl: null }); + expect(window.location.search).toBe(""); + expect(spy).toHaveBeenCalledTimes(2); + }); + + it("preserves other params already on the URL", () => { + setUrl("?page=playlists"); + renderHook(() => useUrlMirror({ pl: "5" })); + expect(window.location.search).toBe("?page=playlists&pl=5"); + }); +});