refactor(frontend): the long tail — and eslint-suppressions.json is gone (E27 S2)
Closes the burn-down. Hook suppressions 113 -> 34, `exhaustive-deps` 45 -> 2 (the target was <10), and every one of the 34 survivors is an inline directive NAMING the invariant it protects. The suppressions file is deleted, because a bulk file hides the next violation instead of answering it — and it cannot hold a warning-level rule at all. The sprint plan's three verdicts missed the answer that turned out to be the most common one: adjust the state DURING RENDER. React re-runs the component and discards that pass rather than committing it, so the stale frame an effect-based reset paints does not exist. Applied to the Plex player's up-next card, the playlist view reset, the feed's optimistic overrides, the channel banner's retry count, the download center's tab guard, the Plex search highlight, the comfort-pin latch, Welcome's per-view outcome and the card pager's page clamp. It is legal only for a component's own state — PlexBrowse's language commit resets PlexProvider's filters, so that one stays an effect and says why. Mirrors of a prop or query became derived values or `draft ?? server`: - `downloadRow` had ui/Avatar's defect exactly — a boolean "poster failed" that an effect reset, in rows recycled down a virtualized list, so one job's missing poster painted over the next one's for a frame. Keyed by URL now. - Scheduler's two number editors held a mirror an effect re-synced, on a page that POLLS — the shape a poll can overwrite mid-edit. A draft while editing, the server's value otherwise; six tests where there were none, the one behavioural change (the save button stops being dirty at commit rather than at the echo) mutation-checked. - SetupWizard's "no token" and WatchPage's "gone" are facts about the URL, so they are seeded and derived rather than pushed into state by an effect that had to commit first. Elsewhere: PlexInfo's rating chip was a component declared during render — a new component type every render, so React remounted the subtree and dropped the focus on its filter button. Hoisted. Feed's undo action and PlexPlayer's session reclaim both re-entered their own useCallback, which the checker reads as a forward reference; both go through the ref idiom this file already uses. BootGate's dep list was load-bearing for correctness — a re-run meant `location.reload()` — so the "already adopted THIS identity" comparison now enforces it in code, with a test that fails without it (the existing reload tests all install their spy too late to catch it). One correction to the batch before this: keying ChatThread's and Messages' decrypt passes on `q.data` instead of `q.dataUpdatedAt` was wrong. react-query's structural sharing makes it safe in production, but the component must not melt down when something hands back a fresh object per render — and Messages.keyboard.dom.test does exactly that, so the effect span the suite. Both are back on the fetch as the trigger with the payload behind a ref, which is honest AND loop-proof; the replaced comment had warned about precisely this. CLAUDE.md gains four canonical-patterns rows for the above, and its "React 18" line is now true.
This commit is contained in:
@@ -290,6 +290,9 @@ export default function Playlists() {
|
||||
});
|
||||
const detail = detailQuery.data;
|
||||
|
||||
// Destructured so the effect below can name what it calls: both are stable useCallbacks, while
|
||||
// the object around them is rebuilt every render (lib/useUndoable).
|
||||
const { reset: resetOrder, rebase: rebaseOrder } = order;
|
||||
useEffect(() => {
|
||||
if (!detail) return;
|
||||
// Scoped to the playlist: two lists holding the SAME videos would otherwise look identical to
|
||||
@@ -297,7 +300,7 @@ export default function Playlists() {
|
||||
const key = `${selectedId}:${idsKey(detail.items)}`;
|
||||
if (key !== lastSetRef.current) {
|
||||
lastSetRef.current = key;
|
||||
order.reset(detail.items);
|
||||
resetOrder(detail.items);
|
||||
} else {
|
||||
// Same membership, fresher ROWS — a watch position the player just stored, a status. The
|
||||
// guard above looks at the id set only, so without this the list kept showing, and handing the
|
||||
@@ -305,17 +308,23 @@ export default function Playlists() {
|
||||
// reopen it, and it resumed at the old spot and the next checkpoint wrote that over the 830.
|
||||
// `rebase` takes the fresh rows into every snapshot and keeps the order and its undo history.
|
||||
const fresh = new Map(detail.items.map((v) => [v.id, v]));
|
||||
order.rebase((list) => list.map((v) => fresh.get(v.id) ?? v));
|
||||
rebaseOrder((list) => list.map((v) => fresh.get(v.id) ?? v));
|
||||
}
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [detail]);
|
||||
// `reset` and `rebase` are stable useCallbacks (lib/useUndoable), so naming them costs nothing;
|
||||
// `selectedId` moves only together with `detail` anyway, since detail IS the selected playlist's.
|
||||
}, [detail, selectedId, resetOrder, rebaseOrder]);
|
||||
|
||||
// Reset the view when switching playlists (the stored order itself is per-playlist).
|
||||
useEffect(() => {
|
||||
// Reset the view when switching playlists (the stored order itself is per-playlist) — during the
|
||||
// render that switches, so the incoming playlist is never drawn once through the previous one's
|
||||
// sort, filters and grouping. All three are this component's own state, which is what makes a
|
||||
// render-phase write legal here.
|
||||
const [viewFor, setViewFor] = useState(selectedId);
|
||||
if (viewFor !== selectedId) {
|
||||
setViewFor(selectedId);
|
||||
setViewSort(STORED_SORT);
|
||||
setFilters({});
|
||||
setGroupBy(false);
|
||||
}, [selectedId]);
|
||||
}
|
||||
|
||||
const builtin = detail?.kind === "watch_later";
|
||||
// YouTube-mirrored playlists can be edited locally and synced back (the read-sync skips a
|
||||
@@ -385,6 +394,7 @@ export default function Playlists() {
|
||||
// Read by the drag announcements, which must report the CURRENT positions without making the
|
||||
// announcement closures a render dependency of DndContext.
|
||||
const shownRef = useRef(shown);
|
||||
// eslint-disable-next-line react-hooks/immutability -- the house latest-value ref: the announcement closures must read the CURRENT list without the list becoming a render dependency of DndContext. A render-phase ref write is never part of the rendered output, which is what makes it safe; the checker is conservative about refs an effect also reads.
|
||||
shownRef.current = shown;
|
||||
|
||||
const viewActive = !isStoredSort(viewSort) || groupBy || filterActive;
|
||||
@@ -595,6 +605,7 @@ export default function Playlists() {
|
||||
}
|
||||
// Kept in a ref so the memoized column callbacks don't rebuild on every state change.
|
||||
const removeRef = useRef(removeItem);
|
||||
// eslint-disable-next-line react-hooks/immutability -- same latest-value ref as shownRef above — it keeps the memoised column callbacks from rebuilding on every state change.
|
||||
removeRef.current = removeItem;
|
||||
|
||||
async function moveItemTo(video: Video, target: Playlist) {
|
||||
|
||||
Reference in New Issue
Block a user