test(e4): readSelectedId ?pl guard, useUrlMirror dep-keying, usePref flush
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).
This commit is contained in:
@@ -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<number | null>(readSelectedId);
|
||||
|
||||
@@ -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<typeof import("../lib/storage")>();
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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<number>("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<number>("vol", 0, { debounceMs: 300 }), {
|
||||
wrapper: wrapperFor(qc),
|
||||
});
|
||||
unmount();
|
||||
expect(savePrefs).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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<typeof vi.spyOn>;
|
||||
|
||||
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<string, string | null>) => useUrlMirror(p), {
|
||||
initialProps: { pl: "5" } as Record<string, string | null>,
|
||||
});
|
||||
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<string, string | null>) => useUrlMirror(p), {
|
||||
initialProps: { pl: "5" } as Record<string, string | null>,
|
||||
});
|
||||
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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user