From d7fb7075b8cc611a484d6d03f8421291b81dc512 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 4 Oct 2026 00:05:37 +0200 Subject: [PATCH] refactor(ui): every function compiles under the React Compiler but VirtualGrid, and react-hooks/refs is on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E32 S4 — the burn-down from 29 skipped functions to 1: - try/finally (26 sites): the cleanup runs after a try/catch; steps that return early moved into their own function (push, the setup wizard's next, the share page's unlock). - A conditional, loop or optional call inside a try skips the function too, and react-hooks/todo does not see it: the value is computed before the try, or the try lives in a module-level helper (config writes, clip queueing, clipboard, frame counts, seekable end, YouTube player times). - The two exhaustive-deps suppressions are gone. useYtProgress takes its per-item snapshot through one effect event, read once when the player is built; PlexPlayer's unmount relies on the leaving-the-item cleanup for the generation bump it duplicated. - What the skips hid: PlexPlayer's 17 render-phase ref writes (useLatest), its tracks popover alias, the loadSession self-reference (declared early, set after commit); PlayerModal's session-position read during render (now where the player starts), navRef/modeRef writes, and a description helper that ran a ref-carrying callback during render (now PlayerDescription); SavedViewsProvider's latest-cfg ref, and its setState in the seed effect (preferSticky now prefers the starred default among same-filter views, so the seed needs no sticky write — test seen red on the old helper). - A store's hook was called `use()`, which the compiler does not know as a hook: inside the newly compiled usePlaylistActions it was memoized, ran on the first render alone, and the playlist rail threw "rendered fewer hooks" on its second render. Renamed to `useValue()` everywhere; the old contract test claimed the call was safe inside any `use…` function, which was false — replaced by a render test that is red under the old name. - PlexPlayer's palette ref was written and never read since the Backspace guard moved to the layer stack: removed. react-hooks/refs is back at error with no exception. The ratchet lists only VirtualGrid (IncompatibleLibrary) and pins both players as compiled. No size or complexity ceiling was raised. --- frontend/eslint.config.js | 40 ++--- frontend/src/components/AddToPlaylist.tsx | 12 +- frontend/src/components/ConfigPanel.tsx | 53 +++--- frontend/src/components/DownloadCenter.tsx | 18 +- frontend/src/components/DownloadDialog.tsx | 3 +- frontend/src/components/KeyGate.tsx | 11 +- frontend/src/components/PlayerModal.tsx | 96 ++++------- frontend/src/components/PlaylistsRail.tsx | 4 +- .../src/components/PlexCollectionEditor.tsx | 16 +- .../src/components/PlexCollectionView.tsx | 16 +- frontend/src/components/PlexPlayer.tsx | 140 ++++++++-------- frontend/src/components/PlexPlaylistAdd.tsx | 12 +- frontend/src/components/PlexStatsOverlay.tsx | 24 +-- .../src/components/SavedViewsProvider.tsx | 29 ++-- frontend/src/components/SettingsPanel.tsx | 11 +- frontend/src/components/SetupWizard.tsx | 64 +++---- frontend/src/components/UploadDialog.tsx | 3 +- frontend/src/components/VideoEditor.tsx | 36 ++-- frontend/src/components/WatchPage.tsx | 68 ++++---- frontend/src/components/Welcome.tsx | 31 ++-- .../compilerCoverage.contract.test.ts | 40 ++--- .../src/components/playerModalControls.tsx | 42 +++++ frontend/src/components/plexViews.tsx | 11 +- .../AnchoredPopover.returnFocus.dom.test.tsx | 3 +- frontend/src/components/ui/backToTopState.ts | 2 +- .../components/ui/useReturnFocus.dom.test.tsx | 6 +- frontend/src/components/useDebugRecording.ts | 4 +- .../src/components/useDownloadActions.tsx | 7 +- .../src/components/usePlaylistActions.tsx | 156 +++++++++--------- .../src/components/usePlexPlaylistEdits.ts | 8 +- frontend/src/lib/debugRecorder.ts | 8 +- frontend/src/lib/homeIntent.ts | 2 +- frontend/src/lib/keymap.ts | 12 +- frontend/src/lib/playerModalHooks.ts | 121 ++++++++------ frontend/src/lib/plexPlayerHooks.ts | 3 +- frontend/src/lib/savedViews.test.ts | 9 + frontend/src/lib/savedViews.ts | 13 +- frontend/src/lib/seekableEnd.ts | 14 ++ frontend/src/lib/store.contract.test.ts | 25 --- frontend/src/lib/store.dom.test.tsx | 51 ++++++ frontend/src/lib/store.ts | 9 +- frontend/src/lib/toastDock.ts | 2 +- 42 files changed, 646 insertions(+), 589 deletions(-) create mode 100644 frontend/src/lib/seekableEnd.ts delete mode 100644 frontend/src/lib/store.contract.test.ts create mode 100644 frontend/src/lib/store.dom.test.tsx diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index 158a3a6a..7a7fe7fa 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -542,9 +542,10 @@ export default tseslint.config( // The React Compiler's own diagnostics that `recommended` leaves out (E32). Each is a function // the compiler SKIPS, silently — `panicThreshold` is "none" and there is no logger, so a lint // error is the only place the skip is said out loud. `todo` is the compiler's "cannot lower - // this shape yet" (31 try statements, each suppressed inline until E32 S3 rewrites them); the - // rest had 0 hits when switched on and are tripwires. The full per-function picture is - // components/compilerCoverage.contract.test.ts. + // this shape yet" (E32 S4 rewrote all 31 try statements it found); the rest had 0 hits when + // switched on and are tripwires. `todo` does NOT see every such shape — a conditional, a loop + // or an optional call inside a try skips the function with no lint hit at all — so the full + // per-function picture is components/compilerCoverage.contract.test.ts. "react-hooks/todo": "error", "react-hooks/memo-dependencies": "error", "react-hooks/rule-suppression": "error", @@ -558,30 +559,17 @@ export default tseslint.config( "react-hooks/exhaustive-effect-dependencies": "error", "react-hooks/no-deriving-state-in-effects": "error", // eslint-plugin-react-hooks 7's `recommended` is no longer two rules — it carries the whole - // React Compiler rule set (E27 S1). On this codebase that landed 283 findings, and they were - // triaged rather than bulk-suppressed, because the two answers are different: + // React Compiler rule set (E27 S1). On this codebase that landed 283 findings. 66 were fixed or + // suppressed inline, each naming its invariant; there is DELIBERATELY no suppressions file — a + // bulk `--suppress-all` would hide the next violation instead of answering it. // - // react-hooks/refs 217 — OFF, see below. - // the other 66 — real. They went into eslint-suppressions.json as a ratchet for one - // sprint and E27 S2 burned the file down to nothing: what survives is - // an inline directive at the site, each naming the invariant it - // protects. There is DELIBERATELY no suppressions file any more — a - // bulk `--suppress-all` would hide the next violation instead of - // answering it, and it cannot hold a warning-level rule anyway. - // - // `refs` is OFF — the one rule here switched off while it has real hits, and recorded as such - // in docs/gates.md. E27 turned it off calling 153 of its 217 hits false positives (the popover - // engine's `pop.refs.setReference` / `getReferenceProps()`, a popover handle passed as a prop) - // and the other ~64 real render-phase `someRef.current = x` writes. The E32 audit showed the - // triage was wrong: the rule and the React Compiler share one analysis, so every one of those - // "false positives" is a component the compiler SKIPS — 21 popover components among them, and - // Channels' 0.9 s row menu was the bill. It stays off only because 245 hits (measured E32 S1) - // could not be fixed in one sprint. E32 S3 took them to 31: the popover engine is read through - // destructuring (`const { setReference } = pop.refs`), every render-phase write moved to - // `useEffectEvent` / `useLatest`. The rest sit in PlexPlayer and PlayerModal, behind the - // try/finally and suppression skips; until they are gone, compilerCoverage.contract.test.ts pins - // every skipped function (none may be added), and this comes back on. - "react-hooks/refs": "off", + // `refs` (the other 217) was switched OFF in E27 as mostly false positives, and that triage was + // wrong: the rule and the React Compiler share one analysis, so every hit was a function the + // compiler SKIPS — 21 popover components among them, and Channels' 0.9 s row menu was the bill + // (E32). E32 S3–S4 fixed every site (245 measured) instead of narrowing the rule: the popover + // engine is read through destructuring, render-phase ref writes became `useEffectEvent` / + // `useLatest`. It is on, at error, with no exception. + "react-hooks/refs": "error", "local/jsx-comment-text": "error", "local/focus-safe-disabled": "error", "local/button-primitive": "error", diff --git a/frontend/src/components/AddToPlaylist.tsx b/frontend/src/components/AddToPlaylist.tsx index 863443e1..4c055fb2 100644 --- a/frontend/src/components/AddToPlaylist.tsx +++ b/frontend/src/components/AddToPlaylist.tsx @@ -119,33 +119,29 @@ function MembershipPanel({ async function toggle(pl: Playlist) { if (busy) return; setBusy(true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { if (pl.has_video) await api.removeFromPlaylist(pl.id, videoId); else await api.addToPlaylist(pl.id, videoId); } catch (e) { notifyActionError(e, t(pl.has_video ? "playlists.removeFailed" : "playlists.addFailed")); - } finally { - refresh(); - setBusy(false); } + refresh(); + setBusy(false); } async function createAndAdd() { const name = newName.trim(); if (!name || busy) return; setBusy(true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { const pl = await api.createPlaylist(name); await api.addToPlaylist(pl.id, videoId); setNewName(""); } catch (e) { notifyActionError(e, t("playlists.addFailed")); - } finally { - refresh(); - setBusy(false); } + refresh(); + setBusy(false); } // All of the user's playlists are add targets, including YouTube-linked ones — adding diff --git a/frontend/src/components/ConfigPanel.tsx b/frontend/src/components/ConfigPanel.tsx index cba7376a..b6211445 100644 --- a/frontend/src/components/ConfigPanel.tsx +++ b/frontend/src/components/ConfigPanel.tsx @@ -45,6 +45,33 @@ function useSeededDraft(baseline: Record, seedKey: string) { return { draft, setDraft, secretReset, setSecretReset }; } +/** The dirty keys' writes, one after another. Apart from ConfigPanel's save() so the loop and its + * branches sit outside the try (the React Compiler cannot lower either inside one yet, and skipped + * the whole panel). */ +async function writeConfigItems( + dirty: ConfigItem[], + draft: Record, + secretReset: Record, +) { + for (const it of dirty) { + const key = it.key; + const raw = draft[key] ?? ""; + if (it.secret) { + if (secretReset[key] && !raw.trim()) await api.resetConfig(key); + else if (raw.trim()) await api.setConfig(key, raw); + } else if (it.type === "bool") { + await api.setConfig(key, raw === "true"); + } else if (raw === "") { + // allow_empty keys treat a blank as an explicit empty value (e.g. disable smtp_user / + // youtube_api_proxy) rather than resetting to the env default (AB3). + if (it.allow_empty) await api.setConfig(key, ""); + else if (it.is_set) await api.resetConfig(key); // cleared → fall back to default + } else { + await api.setConfig(key, it.type === "int" ? Number(raw) : raw); + } + } +} + export default function ConfigPanel() { const { t } = useTranslation(); const panel = useTabPanel(); @@ -54,7 +81,6 @@ export default function ConfigPanel() { const data = q.data; const items = useMemo(() => (data ? Object.values(data.groups).flat() : []), [data]); - const byKey = useMemo(() => Object.fromEntries(items.map((i) => [i.key, i])), [items]); // Baseline draft from the server: non-secrets show their effective value; secrets start empty // (write-only — we never load them back). const baseline = useMemo(() => { @@ -85,30 +111,13 @@ export default function ConfigPanel() { if (it.secret) return (draft[it.key] ?? "").trim() !== "" || !!secretReset[it.key]; return (draft[it.key] ?? "") !== (baseline[it.key] ?? ""); }; - const dirtyKeys = items.filter(isDirty).map((i) => i.key); - const dirty = dirtyKeys.length > 0; + const dirtyItems = items.filter(isDirty); + const dirty = dirtyItems.length > 0; async function save() { setSaveState("saving"); try { - for (const key of dirtyKeys) { - const it = byKey[key]; - if (!it) continue; // dirtyKeys are all config-item keys; skip if somehow not found - const raw = draft[key] ?? ""; - if (it.secret) { - if (secretReset[key] && !raw.trim()) await api.resetConfig(key); - else if (raw.trim()) await api.setConfig(key, raw); - } else if (it.type === "bool") { - await api.setConfig(key, raw === "true"); - } else if (raw === "") { - // allow_empty keys treat a blank as an explicit empty value (e.g. disable smtp_user / - // youtube_api_proxy) rather than resetting to the env default (AB3). - if (it.allow_empty) await api.setConfig(key, ""); - else if (it.is_set) await api.resetConfig(key); // cleared → fall back to default - } else { - await api.setConfig(key, it.type === "int" ? Number(raw) : raw); - } - } + await writeConfigItems(dirtyItems, draft, secretReset); await qc.invalidateQueries({ queryKey: qk.adminConfig() }); // The email-not-configured notice reads me.email_configured, which is otherwise only // refetched on its own 60s interval — without this it keeps warning about fields the admin @@ -190,7 +199,7 @@ export default function ConfigPanel() { // tab pill carries a dot when that group has unsaved edits, so a draft in a hidden tab is // never silently lost behind the global Save bar. const activeGroup = groupKeys.includes(tab) ? tab : groupKeys[0]; - const dirtyByGroup = new Set(dirtyKeys.map((k) => byKey[k]?.group).filter(Boolean)); + const dirtyByGroup = new Set(dirtyItems.map((i) => i.group).filter(Boolean)); const groupTabs = data.sections.flatMap((s) => s.groups.map((g) => ({ id: g, diff --git a/frontend/src/components/DownloadCenter.tsx b/frontend/src/components/DownloadCenter.tsx index 432ba064..21fd324a 100644 --- a/frontend/src/components/DownloadCenter.tsx +++ b/frontend/src/components/DownloadCenter.tsx @@ -45,6 +45,16 @@ import { useDownloadActions, type DownloadTab } from "./useDownloadActions"; const jobKey = (job: DownloadJob) => job.id; +/** The clipboard's text, or null where the browser refuses. Outside the component: the React + * Compiler cannot lower an optional call inside a try yet, and skipped the whole page for it. */ +async function clipboardText(): Promise { + try { + return (await navigator.clipboard?.readText())?.trim() ?? null; + } catch { + return null; // permission denied or unsupported — the manual paste button still works on a click + } +} + export default function DownloadCenter() { const me = useMe(); const { t } = useTranslation(); @@ -115,12 +125,8 @@ export default function DownloadCenter() { // the browser refuses without a click). Only accept an http(s) URL, don't clobber existing text. const pasteFromClipboard = async (force = false) => { if (!force && addUrl.trim()) return; - try { - const text = (await navigator.clipboard?.readText())?.trim(); - if (text && /^https?:\/\/\S+$/i.test(text)) setAddUrl(text); - } catch { - /* permission denied or unsupported — the manual paste button still works on a click */ - } + const text = await clipboardText(); + if (text && /^https?:\/\/\S+$/i.test(text)) setAddUrl(text); }; const tabs = [ diff --git a/frontend/src/components/DownloadDialog.tsx b/frontend/src/components/DownloadDialog.tsx index eab36ba5..b748d631 100644 --- a/frontend/src/components/DownloadDialog.tsx +++ b/frontend/src/components/DownloadDialog.tsx @@ -41,11 +41,12 @@ export default function DownloadDialog({ const submit = async () => { if (!chosen || busy) return; setBusy(true); + const displayName = name.trim() || undefined; try { await api.enqueueDownload({ source, profile_id: chosen, - display_name: name.trim() || undefined, + display_name: displayName, }); invalidateDownloads(qc); notify({ diff --git a/frontend/src/components/KeyGate.tsx b/frontend/src/components/KeyGate.tsx index af4a5258..52716f21 100644 --- a/frontend/src/components/KeyGate.tsx +++ b/frontend/src/components/KeyGate.tsx @@ -44,7 +44,6 @@ export default function KeyGate({ meId, compact = false }: { meId: number; compa if (pass.length < 6) return setErr(t("messages.passTooShort")); if (!configured && pass !== confirm) return setErr(t("messages.passMismatch")); setBusy(true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { if (configured) { await e2ee.unlock(meId, pass, keyQ.data!); @@ -55,9 +54,8 @@ export default function KeyGate({ meId, compact = false }: { meId: number; compa qc.invalidateQueries({ queryKey: qk.messageKey() }); } catch { setErr(configured ? t("messages.wrongPass") : t("messages.setupFailed")); - } finally { - setBusy(false); } + setBusy(false); } function toReset() { @@ -77,9 +75,9 @@ export default function KeyGate({ meId, compact = false }: { meId: number; compa setErr(null); if (hasPassword && !curPass) return setErr(t("messages.resetNeedPass")); setBusy(true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 + const confirmPass = hasPassword ? curPass : undefined; try { - await api.resetMessageKey(hasPassword ? curPass : undefined); + await api.resetMessageKey(confirmPass); // Server key + dead history are gone; wipe this device's identity too so the gate re-appears // in setup mode. Invalidate last so the refetch sees configured:false. await e2ee.resetLocal(meId); @@ -88,9 +86,8 @@ export default function KeyGate({ meId, compact = false }: { meId: number; compa qc.invalidateQueries({ queryKey: qk.messageKey() }); } catch (e) { setErr(e instanceof HttpError && e.detail ? e.detail : t("messages.resetFailed")); - } finally { - setBusy(false); } + setBusy(false); } if (keyQ.isLoading) diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 198d94b7..94983305 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -6,7 +6,6 @@ import { useLayoutEffect, useRef, useState, - type ReactNode, } from "react"; import { useTranslation } from "react-i18next"; import { createPortal } from "react-dom"; @@ -14,7 +13,7 @@ import { useNavigationActions } from "./navigationContext"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; import { usePref } from "../lib/usePref"; -import { PlayerTransportControls } from "./playerModalControls"; +import { PlayerDescription, PlayerTransportControls } from "./playerModalControls"; import { AUTO_MODES, LOOP_MODES, type AutoMode, type LoopMode } from "../lib/playerModes"; import { AlertTriangle, @@ -35,6 +34,7 @@ import { X, } from "lucide-react"; import Avatar from "./ui/Avatar"; +import { useLatest, useStableCallback } from "../lib/useLatest"; import AddToPlaylist from "./AddToPlaylist"; import { PopoverSurface } from "./ui/AnchoredPopover"; import { useAnchoredPopover } from "./ui/anchoredPopoverHooks"; @@ -56,13 +56,11 @@ import { formatCompact, relativeTime, } from "../lib/format"; -import { renderDescription } from "./descriptionLinks"; import { fullscreenElement } from "../lib/fullscreen"; import { dispatch, isActivationTarget, useKeyScope } from "../lib/keymap"; import { useBackToClose } from "../lib/history"; import ChannelNameLink from "./ChannelNameLink"; import Toaster from "./ui/Toaster"; -import { StateMessage } from "./ui/QueryState"; import { useOverlayOrder } from "./ui/overlayOrder"; import { afterRemoval } from "../lib/playerQueue"; import { @@ -185,13 +183,12 @@ export default function PlayerModal({ // The launch queue is frozen at mount, so its items keep the position they were snapshotted with; // without this map, stepping next→prev back to an item played earlier in the SAME session would // resume at that snapshot and then persist it over the newer position. + // A position this session already stored wins over `resumeAt` — it is the freshest truth we have — + // so the map is read where the player starts (useYtProgress, loadVideo), never during render. const sessionPosRef = useRef>(new Map()); - const sessionPos = sessionPosRef.current.get(active.id); - // startAt only applies to the item we opened with; advanced items resume their own pos. A - // position this session already stored wins over both — it is the freshest truth we have. + // startAt only applies to the item we opened with; advanced items resume their own pos. const resumeAt = - sessionPos ?? - (startAt != null && active.id === video.id ? startAt : active.position_seconds || 0); + startAt != null && active.id === video.id ? startAt : active.position_seconds || 0; const mountRef = useRef(null); const playerRef = useRef(null); // Keyboard/wheel controls. We keep focus on the modal card (not the cross-origin player @@ -277,17 +274,20 @@ export default function PlayerModal({ // 100). We overlay a friendly message + "Open on YouTube" instead of YouTube's bare error. const [playerError, setPlayerError] = useState(null); - const loadVideo = (id: string, start: number | null) => { + // Stable callbacks, not plain closures: both reach the player through refs, and the description's + // links get them during render (see PlayerDescription). + const loadVideo = useStableCallback((id: string, start: number | null) => { const p = playerRef.current; if (!p || typeof p.loadVideoById !== "function") return; // Explicit start wins; otherwise resume the active item, start others at 0. - const startSeconds = start != null ? start : id === active.id ? resumeAt : 0; + const startSeconds = + start != null ? start : id === active.id ? (sessionPosRef.current.get(id) ?? resumeAt) : 0; setPlayerError(null); p.loadVideoById({ videoId: id, startSeconds: startSeconds || 0 }); currentIdRef.current = id; setCurrentVideoId(id); setLiveData(null); - }; + }); // Local mirror of watch status so the toggle reacts instantly; changes are propagated to the feed // (and server) via onState. The mirror carries the id it DESCRIBES, and a write for any other id @@ -336,19 +336,18 @@ export default function PlayerModal({ currentIdRef.current = active.id; }, [active.id]); - const seekTo = (seconds: number) => { + const seekTo = useStableCallback((seconds: number) => { const p = playerRef.current; if (p && typeof p.seekTo === "function") { p.seekTo(seconds, true); p.playVideo?.(); } - }; + }); // Prev/next stepping through the queue (the feed's order, or a playlist). Read the live // queue+index from a ref so the window keydown handler (bound once) always steps from the // current position, not a stale closure. - const navRef = useRef({ queue, index }); - navRef.current = { queue, index }; + const navRef = useLatest({ queue, index }); const goPrev = () => { const { queue: q, index: i } = navRef.current; if (q && i > 0) setPlayingId(q[i - 1]!.id); @@ -371,13 +370,9 @@ export default function PlayerModal({ if (!onRemoveFromList || removingRef.current) return; const victim = active; removingRef.current = true; - let done = false; - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try without catch yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 - try { - done = await onRemoveFromList(victim); - } finally { + const done = await onRemoveFromList(victim).finally(() => { removingRef.current = false; - } + }); if (!done) return; // the request failed — leave playback alone // Decide from where the player is NOW, not where it was when the button was pressed: prev/next // and auto-advance keep working while the request runs, and a cursor from before the await @@ -409,8 +404,7 @@ export default function PlayerModal({ // so it seeds from the cache — no localStorage mirror. A ref feeds the (once-bound) end handler. const [autoMode, setAutoMode] = usePref("playerAutoAdvance", "off"); const [loopMode, setLoopMode] = usePref("playerLoop", "off"); - const modeRef = useRef({ autoMode, loopMode }); - modeRef.current = { autoMode, loopMode }; + const modeRef = useLatest({ autoMode, loopMode }); const cycleAuto = () => { setAutoMode(AUTO_MODES[(AUTO_MODES.indexOf(autoMode) + 1) % AUTO_MODES.length]!); }; @@ -609,29 +603,13 @@ export default function PlayerModal({ qc, }); - // The description, said the same way in its two places (the popover under 1280 px, the panel - // beside the video above it): loading, failed with a retry, the text, or none. One ladder, so the - // two cannot drift (E31 review 2: the failed state was tested in the panel only). `wrap` is each - // place's own box for the text. - const description = (wrap: (body: ReactNode) => ReactNode) => - detail.isLoading ? ( -
{t("player.loading")}
- ) : detail.isError && !detail.data ? ( - void detail.refetch()}> - {t("player.descriptionFailed")} - - ) : detail.data?.description ? ( - wrap( - renderDescription(detail.data.description, { - currentId: currentVideoId, - onSeek: seekTo, - onLoadVideo: loadVideo, - t, - }), - ) - ) : ( -
{t("player.noDescription")}
- ); + // The description's two places share one ladder (PlayerDescription) and these props. + const descriptionProps = { + detail, + currentId: currentVideoId, + onSeek: seekTo, + onLoadVideo: loadVideo, + }; // Portaled to , like every other full-screen overlay here. A fixed, full-viewport modal // must not render inside the page scroller: once that scroller is a stacking context (its @@ -916,15 +894,12 @@ export default function PlayerModal({ {t("player.description")} - {description((body) => ( -
- {body} -
- ))} + ); diff --git a/frontend/src/components/useDebugRecording.ts b/frontend/src/components/useDebugRecording.ts index 0e8c5d37..80837101 100644 --- a/frontend/src/components/useDebugRecording.ts +++ b/frontend/src/components/useDebugRecording.ts @@ -106,16 +106,14 @@ export function useDebugRecording() { if (!session) return; hide(true); let image: Blob | null = null; - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { // Two frames: the pill's `invisible` must be painted before the capture reads the tab. await new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(r))); image = await captureTab(); } catch (err) { notifyActionError(err, t("recorder.pill.shotFailed")); - } finally { - hide(false); } + hide(false); if (image) shoot.mutate({ id: session.id, image }); }; diff --git a/frontend/src/components/useDownloadActions.tsx b/frontend/src/components/useDownloadActions.tsx index 4726d893..23e7a70d 100644 --- a/frontend/src/components/useDownloadActions.tsx +++ b/frontend/src/components/useDownloadActions.tsx @@ -89,12 +89,7 @@ export function useDownloadActions({ const quickShare = async (job: DownloadJob) => { if (sharingId !== null) return; setSharingId(job.id); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try without catch yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 - try { - await quickShareLink(qc, job, t); - } finally { - setSharingId(null); - } + await quickShareLink(qc, job, t).finally(() => setSharingId(null)); }; const icon = (I: typeof Play) => ; diff --git a/frontend/src/components/usePlaylistActions.tsx b/frontend/src/components/usePlaylistActions.tsx index 2938e658..57248215 100644 --- a/frontend/src/components/usePlaylistActions.tsx +++ b/frontend/src/components/usePlaylistActions.tsx @@ -59,7 +59,7 @@ export function usePlaylistActions() { const copy = useCopyItems(); const { selectedId } = usePlaylists(); const { setSelectedId } = usePlaylistsActions(); - const { pushing, reverting } = inFlight.use(); + const { pushing, reverting } = inFlight.useValue(); const refresh = (id: number) => { qc.invalidateQueries({ queryKey: qk.playlists() }); @@ -131,7 +131,6 @@ export function usePlaylistActions() { // Re-read after the dialog: another menu may have started one while this one asked. if (!ok || syncing(pl.id)) return; mark("reverting", pl.id, true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { await api.revertPlaylist(pl.id); refresh(pl.id); @@ -141,91 +140,94 @@ export function usePlaylistActions() { }); } catch (e) { notifyYouTubeActionError(e, t("playlists.revertFailed")); - } finally { - mark("reverting", pl.id, false); } + mark("reverting", pl.id, false); } async function push(pl: Menuable) { if (syncing(pl.id)) return; const name = playlistName(pl, t); mark("pushing", pl.id, true); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 try { - const plan = await api.playlistPushPlan(pl.id); - const nothingToDo = - plan.action === "update" && - !plan.add_yt && - !plan.add_local && - !plan.remove_yt && - !plan.remove_local && - !plan.to_reorder && - !plan.rename; - if (nothingToDo) { - notify({ level: "info", message: t("playlists.pushUpToDate", { name }) }); - return; - } - if (!plan.affordable) { - notify({ - level: "warning", - message: t("playlists.pushNoQuota", { - left: plan.remaining_today, - units: plan.units_estimate, - }), - }); - return; - } - let message: string; - if (plan.action === "create") { - message = t("playlists.pushPlanCreate", { - count: plan.add_yt, - units: plan.units_estimate, - left: plan.remaining_today, - }); - } else { - // One sentence per side that actually changes; the untouched side gets a short "stays - // unchanged" reassurance instead of a "+0 / −0" the reader has to decode. - const here: string[] = []; - if (plan.add_local) here.push(t("playlists.mergeAddLocal", { count: plan.add_local })); - if (plan.remove_local) - here.push(t("playlists.mergeRemoveLocal", { count: plan.remove_local })); - const yt: string[] = []; - if (plan.add_yt) yt.push(t("playlists.mergeAddYt", { count: plan.add_yt })); - if (plan.remove_yt) yt.push(t("playlists.mergeRemoveYt", { count: plan.remove_yt })); - if (plan.to_reorder) yt.push(t("playlists.mergeReorder", { count: plan.to_reorder })); - if (plan.rename) yt.push(t("playlists.mergeRename")); - const sentences = [...here, ...yt]; - if (here.length && !yt.length) sentences.push(t("playlists.mergeYtUnchanged")); - if (yt.length && !here.length) sentences.push(t("playlists.mergeLocalUnchanged")); - const quota = - plan.units_estimate > 0 - ? t("playlists.mergeQuota", { - units: plan.units_estimate, - left: plan.remaining_today, - }) - : t("playlists.mergeQuotaFree"); - message = sentences.join(" ") + quota; - } - const ok = await confirm({ - title: t("playlists.pushTitle"), - message, - confirmLabel: t("playlists.pushConfirm"), - }); - if (!ok) return; - const r = await api.pushPlaylist(pl.id); - refresh(pl.id); - if (r.failures.length) { - notify({ - level: "warning", - message: t("playlists.pushPartial", { count: r.failures.length }), - }); - } else { - notify({ level: "success", message: t("playlists.pushDone", { name }) }); - } + await pushSteps(pl, name); } catch (e) { notifyYouTubeActionError(e, t("playlists.pushFailed")); - } finally { - mark("pushing", pl.id, false); + } + mark("pushing", pl.id, false); + } + + /** push()'s steps, apart so its "pushing" mark clears on every way out without a try/finally + * (the React Compiler cannot lower one yet, and skips the whole hook). */ + async function pushSteps(pl: Menuable, name: string) { + const plan = await api.playlistPushPlan(pl.id); + const nothingToDo = + plan.action === "update" && + !plan.add_yt && + !plan.add_local && + !plan.remove_yt && + !plan.remove_local && + !plan.to_reorder && + !plan.rename; + if (nothingToDo) { + notify({ level: "info", message: t("playlists.pushUpToDate", { name }) }); + return; + } + if (!plan.affordable) { + notify({ + level: "warning", + message: t("playlists.pushNoQuota", { + left: plan.remaining_today, + units: plan.units_estimate, + }), + }); + return; + } + let message: string; + if (plan.action === "create") { + message = t("playlists.pushPlanCreate", { + count: plan.add_yt, + units: plan.units_estimate, + left: plan.remaining_today, + }); + } else { + // One sentence per side that actually changes; the untouched side gets a short "stays + // unchanged" reassurance instead of a "+0 / −0" the reader has to decode. + const here: string[] = []; + if (plan.add_local) here.push(t("playlists.mergeAddLocal", { count: plan.add_local })); + if (plan.remove_local) + here.push(t("playlists.mergeRemoveLocal", { count: plan.remove_local })); + const yt: string[] = []; + if (plan.add_yt) yt.push(t("playlists.mergeAddYt", { count: plan.add_yt })); + if (plan.remove_yt) yt.push(t("playlists.mergeRemoveYt", { count: plan.remove_yt })); + if (plan.to_reorder) yt.push(t("playlists.mergeReorder", { count: plan.to_reorder })); + if (plan.rename) yt.push(t("playlists.mergeRename")); + const sentences = [...here, ...yt]; + if (here.length && !yt.length) sentences.push(t("playlists.mergeYtUnchanged")); + if (yt.length && !here.length) sentences.push(t("playlists.mergeLocalUnchanged")); + const quota = + plan.units_estimate > 0 + ? t("playlists.mergeQuota", { + units: plan.units_estimate, + left: plan.remaining_today, + }) + : t("playlists.mergeQuotaFree"); + message = sentences.join(" ") + quota; + } + const ok = await confirm({ + title: t("playlists.pushTitle"), + message, + confirmLabel: t("playlists.pushConfirm"), + }); + if (!ok) return; + const r = await api.pushPlaylist(pl.id); + refresh(pl.id); + if (r.failures.length) { + notify({ + level: "warning", + message: t("playlists.pushPartial", { count: r.failures.length }), + }); + } else { + notify({ level: "success", message: t("playlists.pushDone", { name }) }); } } diff --git a/frontend/src/components/usePlexPlaylistEdits.ts b/frontend/src/components/usePlexPlaylistEdits.ts index 0603d8cf..925e8327 100644 --- a/frontend/src/components/usePlexPlaylistEdits.ts +++ b/frontend/src/components/usePlexPlaylistEdits.ts @@ -96,7 +96,7 @@ export function usePlexPlaylistEdits(playlistId: number) { const before = items.map((i) => i.id); const set = new Set(keys); setItems((prev) => prev.filter((i) => !set.has(i.id))); - // eslint-disable-next-line react-hooks/todo -- the React Compiler cannot lower try/finally yet, so the enclosing component or hook stays uncompiled; pinned in compilerCoverage, burned down in E32 S3 + let removed = true; try { if (keys.length === 1) await api.plexPlaylistRemoveItem(playlistId, keys[0]!); else await api.plexPlaylistRemoveBulk(playlistId, keys); @@ -104,10 +104,10 @@ export function usePlexPlaylistEdits(playlistId: number) { } catch (e) { notifyActionError(e, t("plex.playlist.removeFailed")); putBack(); - return; - } finally { - setBusy(false); + removed = false; } + setBusy(false); + if (!removed) return; notify({ level: "success", message: t("plex.playlist.removedToast", { title }), diff --git a/frontend/src/lib/debugRecorder.ts b/frontend/src/lib/debugRecorder.ts index e8e266a7..4f0ec654 100644 --- a/frontend/src/lib/debugRecorder.ts +++ b/frontend/src/lib/debugRecorder.ts @@ -8,14 +8,14 @@ const startOpen = createStore(false); /** Open the start dialog (Settings → Diagnosztika, the palette). */ export const openRecorderStart = (): void => startOpen.set(true); export const closeRecorderStart = (): void => startOpen.set(false); -export const useRecorderStartOpen = (): boolean => startOpen.use(); +export const useRecorderStartOpen = (): boolean => startOpen.useValue(); // The pill's step prompt — open, and what is typed in it — held here, not in the pill: the pill // rides the topmost toast host, and a dialog or the player opening or closing moves it to another // one (a remount). The prompt must come along with what was typed. const stepPrompt = createStore(null); /** The step prompt's text while it is open, else null. */ -export const useStepPrompt = (): string | null => stepPrompt.use(); +export const useStepPrompt = (): string | null => stepPrompt.useValue(); export const setStepPrompt = (text: string | null): void => stepPrompt.set(text); // Other tabs of this browser: a Start or a Stop in one is news to the rest, which otherwise learn of @@ -42,7 +42,7 @@ const composing = createStore(null); /** Open the issue composer for session `id` — after Stop, when the cap ends it, from Settings. */ export const openComposer = (id: string): void => composing.set(id); export const closeComposer = (): void => composing.set(null); -export const useComposerSession = (): string | null => composing.use(); +export const useComposerSession = (): string | null => composing.useValue(); // Step markers are numbered per session. The count survives a reload (sessionStorage, per tab): a // reproduction is often "reload and it happens again", and step 4 coming back as step 1 would make @@ -61,7 +61,7 @@ function readSteps(): Record { const steps = createStore>(readSteps()); /** How many steps session `id` has marked in this tab. */ -export const useStepCount = (id: string): number => steps.use()[id] ?? 0; +export const useStepCount = (id: string): number => steps.useValue()[id] ?? 0; /** Take the next step number for session `id`. */ export function nextStep(id: string): number { diff --git a/frontend/src/lib/homeIntent.ts b/frontend/src/lib/homeIntent.ts index 4b8e8c44..ad20cee1 100644 --- a/frontend/src/lib/homeIntent.ts +++ b/frontend/src/lib/homeIntent.ts @@ -20,7 +20,7 @@ const surprise = createStore(null); export const requestSurprise = (): void => surprise.set(ensureNavId()); /** The pending token (null = nothing pending) — the render subscription that wakes Home up. Home only. */ -export const useSurpriseRequest = (): number | null => surprise.use(); +export const useSurpriseRequest = (): number | null => surprise.useValue(); /** Take the pending request, clearing it; true only when it was requested for the entry Home is on. * diff --git a/frontend/src/lib/keymap.ts b/frontend/src/lib/keymap.ts index 50c1d332..f37990a9 100644 --- a/frontend/src/lib/keymap.ts +++ b/frontend/src/lib/keymap.ts @@ -416,7 +416,7 @@ const tokens: { scope: KeyScope }[] = []; /** The live scopes, innermost first, always ending in `global`. Re-renders on change. */ export function useActiveScopes(): KeyScope[] { - const stack = scopeStack.use(); + const stack = scopeStack.useValue(); // DEDUPED: a scope can be registered by more than one live component (the Playlists page and its // rail each mount a `useContextMenu`), and `helpLines` appends a row's chords once per occurrence — // so without this the sheet rendered "⇧ F10 Menu ⇧ F10 Menu" under a duplicated React key @@ -449,14 +449,14 @@ export const toggleKeyboardHelp = (): void => { helpOpen.set(next); }; /** Subscribe to whether the sheet is showing (the Plex player suppresses wheel-volume under it). */ -export const useKeyboardHelpOpen = (): boolean => helpOpen.use(); +export const useKeyboardHelpOpen = (): boolean => helpOpen.useValue(); /** Whether the sheet was opened over a keyboard-owning layer (see `helpOverLayer`). */ -export const useKeyboardHelpOverLayer = (): boolean => helpOverLayer.use(); +export const useKeyboardHelpOverLayer = (): boolean => helpOverLayer.useValue(); // The modules this account can reach — published by `useGlobalKeymap`, read by the `g` chords and // by the sheet (which drops the chord of a gated module rather than promise a page that isn't there). const navigablePages = createStore([]); -export const useNavigablePages = (): readonly Page[] => navigablePages.use(); +export const useNavigablePages = (): readonly Page[] => navigablePages.useValue(); // ── The command palette ─────────────────────────────────────────────────────────────────────── const paletteOpen = createStore(false); @@ -464,7 +464,7 @@ const paletteOpen = createStore(false); export const closePalette = (): void => paletteOpen.set(false); /** Open it from a visible control (the header's key cap / launcher), not only from Ctrl/Cmd+K. */ export const openPalette = (): void => paletteOpen.set(true); -export const usePaletteOpen = (): boolean => paletteOpen.use(); +export const usePaletteOpen = (): boolean => paletteOpen.useValue(); /** Bind the `global` and `page` scopes, once, for the whole app. It lives here rather than inside * either overlay because both are opened by it: a listener that sat in one of them would have to @@ -576,7 +576,7 @@ export function useSearchFocusKey(input: RefObject): vo }, [input]); } -export const useHelpConditionValues = (): HelpConditions => helpConditions.use(); +export const useHelpConditionValues = (): HelpConditions => helpConditions.useValue(); // ── Display ─────────────────────────────────────────────────────────────────────────────────── const GLYPHS: Record = { diff --git a/frontend/src/lib/playerModalHooks.ts b/frontend/src/lib/playerModalHooks.ts index 40baad1a..d2773848 100644 --- a/frontend/src/lib/playerModalHooks.ts +++ b/frontend/src/lib/playerModalHooks.ts @@ -315,6 +315,21 @@ export function useYtBigView({ }; } +/** The player's position and length, or null when there is no usable player — not built yet, or + * tearing down (its methods throw then). Outside the hook: the React Compiler cannot lower a + * conditional inside a try yet, and skipped useYtProgress for it. */ +function playerTimes(p: YTPlayer | null): { cur: number; dur: number } | null { + if (!p || typeof p.getCurrentTime !== "function") return null; + try { + return { + cur: p.getCurrentTime(), + dur: typeof p.getDuration === "function" ? p.getDuration() : 0, + }; + } catch { + return null; + } +} + /** Create the YouTube IFrame player for the active item, resume from the saved position, checkpoint * progress every 5 s (+ a pagehide beacon), and auto-mark watched once playback reaches the end. Keyed * on activeId (a queue advance recreates the player to reuse this single-video logic). The once-bound @@ -358,10 +373,34 @@ export function useYtProgress({ }): void { const autoMarkedRef = useRef(false); const errorStreakRef = useRef(0); + // The render values this player is built from, read ONCE when it is built (see the dep note at + // the end of the effect). + const itemSnapshot = useEffectEvent(() => ({ + resumeAt, + setWatched, + setPlayerError, + setLiveData, + reapplyStored, + focusModal, + advanceOnEnd, + qc, + })); useEffect(() => { let cancelled = false; const id = activeId; + const { + resumeAt, + setWatched, + setPlayerError, + setLiveData, + reapplyStored, + focusModal, + advanceOnEnd, + qc, + } = itemSnapshot(); + // A position this session already stored for the item beats the queue's snapshot of it. + const startAt = sessionPosRef.current.get(id) ?? resumeAt; autoMarkedRef.current = false; setPlayerError(null); @@ -379,28 +418,23 @@ export function useYtProgress({ return false; }; const persist = (): Promise | void => { - const p = playerRef.current; - if (!p || typeof p.getCurrentTime !== "function") return; - try { - const cur = p.getCurrentTime(); - const dur = typeof p.getDuration === "function" ? p.getDuration() : 0; - // If we just auto-marked watched, skip the near-end checkpoint (it would only clear the resume - // position, racing the watched write against the progress row it deletes). - if (maybeAutoWatch(cur, dur)) return; - // Only checkpoint the library item we opened with — a navigated-to video may not be in this - // user's library, so the server would 404 on it. - if (currentIdRef.current === id) { - return ( - api - .saveProgress(id, cur, dur) - // Record what the server STORED, not `cur`: a near-end or under-5s checkpoint is - // cleared to 0 there, and the resume must agree with the row, not with our reading. - .then((res) => void sessionPosRef.current.set(id, res.position_seconds)) - .catch(() => {}) - ); - } - } catch { - /* player may be tearing down */ + const times = playerTimes(playerRef.current); + if (!times) return; + const { cur, dur } = times; + // If we just auto-marked watched, skip the near-end checkpoint (it would only clear the resume + // position, racing the watched write against the progress row it deletes). + if (maybeAutoWatch(cur, dur)) return; + // Only checkpoint the library item we opened with — a navigated-to video may not be in this + // user's library, so the server would 404 on it. + if (currentIdRef.current === id) { + return ( + api + .saveProgress(id, cur, dur) + // Record what the server STORED, not `cur`: a near-end or under-5s checkpoint is + // cleared to 0 there, and the resume must agree with the row, not with our reading. + .then((res) => void sessionPosRef.current.set(id, res.position_seconds)) + .catch(() => {}) + ); } }; @@ -413,7 +447,7 @@ export function useYtProgress({ videoId: id, playerVars: { autoplay: 1, - start: resumeAt || undefined, + start: startAt || undefined, rel: 0, // limit "related" to the same channel (full removal is no longer possible) enablejsapi: 1, origin: window.location.origin, @@ -480,15 +514,9 @@ export function useYtProgress({ // Save on page unload too (F5/close/navigate): effect cleanup doesn't run on a full reload. A // keepalive beacon so the POST survives the unload. Only for the library item we opened with. const onHide = () => { - const p = playerRef.current; - if (!p || typeof p.getCurrentTime !== "function" || currentIdRef.current !== id) return; - try { - const cur = p.getCurrentTime(); - const dur = typeof p.getDuration === "function" ? p.getDuration() : 0; - if (cur > 0 && dur > 0) api.saveProgressBeacon(id, cur, dur); - } catch { - /* player tearing down */ - } + if (currentIdRef.current !== id) return; + const times = playerTimes(playerRef.current); + if (times && times.cur > 0 && times.dur > 0) api.saveProgressBeacon(id, times.cur, times.dur); }; window.addEventListener("pagehide", onHide); @@ -517,21 +545,18 @@ export function useYtProgress({ } playerRef.current = null; }; - // ONE PLAYER PER VIDEO is the invariant, and it is why this dep list is deliberately just the - // id: re-running would destroy and rebuild the YouTube iframe, losing the playing position and - // every listener attached to it. + // ONE PLAYER PER VIDEO is the invariant: re-running would destroy and rebuild the YouTube + // iframe, losing the playing position and every listener attached to it. So the deps are the id + // and refs (which never change). // - // This body DOES capture render values — `resumeAt`, `setWatched`, `advanceOnEnd`, - // `reapplyStored`, `focusModal`, `qc` — and it must, because each of them is a PER-ITEM - // SNAPSHOT: they describe the video this player was built for, and re-reading them mid-session - // would be the bug, not the fix. `resumeAt` is the clearest case, since persist() writes - // sessionPosRef and so changes it while `activeId` has not moved; the player is already playing - // from it and must not be told again. What genuinely has to stay live mid-session is behind refs - // (currentIdRef, modeRef, navRef, sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) — and - // THAT split is the enforcement: a value that is not a snapshot does not belong in this closure. - // - // useEffectEvent cannot help here the way it does above: YT.Player takes its callbacks as an - // object literal, and an effect event may not be passed around as a value. - // eslint-disable-next-line react-hooks/exhaustive-deps -- ONE player per video: the captured values are per-item snapshots, live ones are refs - }, [activeId]); + // The render values the body uses — `resumeAt`, `setWatched`, `advanceOnEnd`, `reapplyStored`, + // `focusModal`, `qc`, … — are a PER-ITEM SNAPSHOT, taken once through `itemSnapshot` when this + // player is built: they describe the video it was built for, and re-reading them mid-session + // would be the bug, not the fix. `setWatched` is the clearest case: bound to the item, it lets + // this cleanup's late auto-mark land on the item being LEFT, which the modal then ignores. What + // genuinely has to stay live mid-session is behind refs (currentIdRef, modeRef, navRef, + // sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) — and THAT split is the enforcement: + // a value that is not a snapshot does not belong in this closure. (E32: this replaced an + // exhaustive-deps suppression, which made the React Compiler skip the whole hook.) + }, [activeId, mountRef, playerRef, currentIdRef, navRef, modeRef, sessionPosRef]); } diff --git a/frontend/src/lib/plexPlayerHooks.ts b/frontend/src/lib/plexPlayerHooks.ts index e347319e..d1d04120 100644 --- a/frontend/src/lib/plexPlayerHooks.ts +++ b/frontend/src/lib/plexPlayerHooks.ts @@ -670,8 +670,9 @@ export function usePlexTracks(opts: { }); if (multiAudioRef.current && hlsRef.current) { // Multi-rendition: switch the audio track CLIENT-SIDE — instant, same timeline, no restart. + const track = ord ?? 0; try { - hlsRef.current.audioTrack = ord ?? 0; + hlsRef.current.audioTrack = track; } catch { /* ignore */ } diff --git a/frontend/src/lib/savedViews.test.ts b/frontend/src/lib/savedViews.test.ts index c1cce3c6..0bb8e35e 100644 --- a/frontend/src/lib/savedViews.test.ts +++ b/frontend/src/lib/savedViews.test.ts @@ -19,6 +19,15 @@ describe("preferSticky", () => { expect(preferSticky([v(1), v(2)], 2)?.id).toBe(2); }); + it("without a sticky match, prefers the starred default over an earlier twin", () => { + // The first-load seed applies the default's filters and leaves the name to this tie-break: an + // earlier view with the same filters must not take the default's name (E32). + const d = (id: number, is_default = false) => ({ id, name: `v${id}`, is_default }); + expect(preferSticky([d(1), d(2, true)], null)?.id).toBe(2); + expect(preferSticky([d(1), d(2, true)], 99)?.id).toBe(2); + expect(preferSticky([d(1), d(2, true)], 1)?.id).toBe(1); + }); + it("falls back to the first match when the sticky id isn't among the matches", () => { expect(preferSticky([v(1), v(2)], 99)?.id).toBe(1); expect(preferSticky([v(1), v(2)], null)?.id).toBe(1); diff --git a/frontend/src/lib/savedViews.ts b/frontend/src/lib/savedViews.ts index 0902fa5b..eaafc7c7 100644 --- a/frontend/src/lib/savedViews.ts +++ b/frontend/src/lib/savedViews.ts @@ -3,12 +3,19 @@ /** Among the views whose filters match the live filters, pick the one to treat as active: prefer the * explicitly-applied ("sticky") view when several capture identical filters, so apply(B) isn't - * silently re-attributed to an earlier duplicate A; otherwise the first match. Null if none match. */ -export function preferSticky( + * silently re-attributed to an earlier duplicate A; else the starred default, which is what the + * first-load seed applies (so the seed needs no sticky write of its own); else the first match. + * Null if none match. */ +export function preferSticky( matches: T[], stickyId: number | null, ): T | null { - return matches.find((v) => v.id === stickyId) ?? matches[0] ?? null; + return ( + matches.find((v) => v.id === stickyId) ?? + matches.find((v) => v.is_default) ?? + matches[0] ?? + null + ); } /** The starred default view to seed a module's filters with on first load, or null to leave them. diff --git a/frontend/src/lib/seekableEnd.ts b/frontend/src/lib/seekableEnd.ts new file mode 100644 index 00000000..11ed91e8 --- /dev/null +++ b/frontend/src/lib/seekableEnd.ts @@ -0,0 +1,14 @@ +/** Where the media can be sought to, on the session's absolute clock (`now` while the element has + * no range yet), or null when the element refuses to say. Its own function, outside PlexPlayer: the + * React Compiler cannot lower a conditional inside a try yet, and skipped the whole player for it. */ +export function seekableEndOf( + video: HTMLVideoElement, + sessionStart: number, + now: number, +): number | null { + try { + return video.seekable.length ? sessionStart + video.seekable.end(0) : now; + } catch { + return null; + } +} diff --git a/frontend/src/lib/store.contract.test.ts b/frontend/src/lib/store.contract.test.ts deleted file mode 100644 index 04e397b4..00000000 --- a/frontend/src/lib/store.contract.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { readdirSync, readFileSync } from "node:fs"; -import { describe, expect, it } from "vitest"; - -// A store's `.use()` is a hook the React Compiler cannot see: its name is a method's, not `use…`'s. -// Called straight from a COMPONENT, the compiled render memoized its result and never re-rendered on -// a change — Back to top stayed hidden (E28 UAT, measured under vitest's compiler build). Inside a -// function named `use…` it is compiled as a hook and works, so that is the only place it may be read. -describe("a store's use()", () => { - it("is read only inside a function named use…", () => { - const root = new URL("../", import.meta.url); - const offenders: string[] = []; - for (const f of readdirSync(root, { recursive: true, encoding: "utf8" })) { - if (!/\.tsx?$/.test(f) || f.includes(".test.")) continue; - const src = readFileSync(new URL(f.replace(/\\/g, "/"), root), "utf8"); - for (const m of src.matchAll(/\b\w+\.use\(\)/g)) { - const before = src.slice(0, m.index); - const owners = [...before.matchAll(/(?:function\s+(\w+)|const\s+(\w+)\s*=\s*\()/g)]; - const owner = owners[owners.length - 1]; - const name = owner?.[1] ?? owner?.[2] ?? ""; - if (!name.startsWith("use")) offenders.push(`${f}: ${m[0]} in ${name || "?"}`); - } - } - expect(offenders).toEqual([]); - }); -}); diff --git a/frontend/src/lib/store.dom.test.tsx b/frontend/src/lib/store.dom.test.tsx new file mode 100644 index 00000000..9d54f6e3 --- /dev/null +++ b/frontend/src/lib/store.dom.test.tsx @@ -0,0 +1,51 @@ +// @vitest-environment jsdom +// +// A store is read from React through `useValue()`, a name the React Compiler knows as a hook. Its +// old name, `use()`, it did not: the call was compiled as an ordinary one and memoized, so it ran +// on the first render alone. Read straight from a component, Back to top never re-rendered (E28); +// read inside a hook that compiles memo scopes around it, usePlaylistActions rendered fewer hooks +// on its second render and threw (E32 S4, the moment that hook started to compile). Vitest builds +// with the same compiler as the app (reactPlugin), so this renders the compiled code. +import { describe, it, expect, afterEach } from "vitest"; +import { render, screen, act, cleanup } from "@testing-library/react"; +import { useState } from "react"; +import { createStore } from "./store"; + +afterEach(cleanup); + +const count = createStore(0); + +// A hook shaped like usePlaylistActions: other hooks around the read, and a callback built from it. +function useCountLabel(prefix: string) { + const [step] = useState(1); + const value = count.useValue(); + const next = () => count.set(value + step); + return { label: `${prefix}${value}`, next }; +} + +function Counter() { + const { label, next } = useCountLabel("n="); + return ; +} + +function Bare() { + return {`bare=${count.useValue()}`}; +} + +describe("a store read from React", () => { + it("re-renders a compiled hook and a compiled component on every change", () => { + act(() => count.set(0)); + render( + <> + + + , + ); + act(() => count.set(1)); + expect(screen.getByRole("button").textContent).toBe("n=1"); + expect(screen.getByText("bare=1")).toBeTruthy(); + act(() => screen.getByRole("button").click()); + expect(screen.getByRole("button").textContent).toBe("n=2"); + expect(screen.getByText("bare=2")).toBeTruthy(); + }); +}); diff --git a/frontend/src/lib/store.ts b/frontend/src/lib/store.ts index 8f68e9a4..83008505 100644 --- a/frontend/src/lib/store.ts +++ b/frontend/src/lib/store.ts @@ -7,8 +7,11 @@ export interface Store { get(): T; set(next: T | ((prev: T) => T)): void; subscribe(listener: () => void): () => void; - /** Subscribe from a component; re-renders on every change. */ - use(): T; + /** Subscribe from a component; re-renders on every change. Named `use…` on purpose: the React + * Compiler knows a hook only by that name, and a plain `use()` was compiled as an ordinary call — + * memoized, so it ran on the first render alone (E32: usePlaylistActions then rendered fewer hooks + * and threw; E28: Back to top never re-rendered). */ + useValue(): T; } export function createStore(initial: T): Store { @@ -28,6 +31,6 @@ export function createStore(initial: T): Store { listeners.forEach((l) => l()); }, subscribe, - use: () => useSyncExternalStore(subscribe, get, get), + useValue: () => useSyncExternalStore(subscribe, get, get), }; } diff --git a/frontend/src/lib/toastDock.ts b/frontend/src/lib/toastDock.ts index 35665d5a..394b3680 100644 --- a/frontend/src/lib/toastDock.ts +++ b/frontend/src/lib/toastDock.ts @@ -24,4 +24,4 @@ export function dockUnderToasts(component: ComponentType): () => void { } /** What is docked (the Toaster reads it). */ -export const useToastDock = (): ComponentType | null => docked.use(); +export const useToastDock = (): ComponentType | null => docked.useValue();