peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(ui): every function compiles under the React Compiler but VirtualGrid, and react-hooks/refs is on

Browse Source 
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.
This commit is contained in:
peter
2026-10-04 00:05:37 +02:00
parent a486c7521a
commit d7fb7075b8
42 changed files with 646 additions and 589 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/debugRecorder.ts
+4 -4
View File
@@ -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<string | null>(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<string | null>(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<string, number> {
const steps = createStore<Record<string, number>>(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 {
frontend/src/lib/homeIntent.ts
+1 -1
View File
@@ -20,7 +20,7 @@ const surprise = createStore<number | null>(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.
*
frontend/src/lib/keymap.ts
+6 -6
View File
@@ -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<readonly Page[]>([]);
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<HTMLInputElement | null>): vo
}, [input]);
}
export const useHelpConditionValues = (): HelpConditions => helpConditions.use();
export const useHelpConditionValues = (): HelpConditions => helpConditions.useValue();
// ── Display ───────────────────────────────────────────────────────────────────────────────────
const GLYPHS: Record<string, string> = {
frontend/src/lib/playerModalHooks.ts
+73 -48
View File
@@ -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<unknown> | 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]);
}
frontend/src/lib/plexPlayerHooks.ts
+2 -1
View File
@@ -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 */
}
frontend/src/lib/savedViews.test.ts
+9
View File
@@ -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);
frontend/src/lib/savedViews.ts
+10 -3
View File
@@ -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<T extends { id: number }>(
* 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<T extends { id: number; is_default?: boolean }>(
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.
frontend/src/lib/seekableEnd.ts
+14
View File
@@ -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;
}
}
frontend/src/lib/store.contract.test.ts
-25
View File
@@ -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([]);
});
});
frontend/src/lib/store.dom.test.tsx
+51
View File
@@ -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 <button onClick={next}>{label}</button>;
}
function Bare() {
return <span>{`bare=${count.useValue()}`}</span>;
}
describe("a store read from React", () => {
it("re-renders a compiled hook and a compiled component on every change", () => {
act(() => count.set(0));
render(
<>
<Counter />
<Bare />
</>,
);
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();
});
});
frontend/src/lib/store.ts
+6 -3
View File
@@ -7,8 +7,11 @@ export interface Store<T> {
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<T>(initial: T): Store<T> {
@@ -28,6 +31,6 @@ export function createStore<T>(initial: T): Store<T> {
listeners.forEach((l) => l());
},
subscribe,
use: () => useSyncExternalStore(subscribe, get, get),
useValue: () => useSyncExternalStore(subscribe, get, get),
};
}
frontend/src/lib/toastDock.ts
+1 -1
View File
@@ -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();