refactor: hook dependencies name everything the hooks read
The 12 react-hooks/exhaustive-deps warnings the gate let through: - ShareDialog and YearFilter built a fresh [] per render while their query loaded, so their memos recomputed on every render; they share one empty list now. - VideoEditor's drag effect called a seek/moveBoundary recreated each render; both are stable callbacks in its deps. - WizardProvider re-checks onboarding on exactly the fields it reads (not the me object, whose 60 s refetch would reopen a closed wizard). - Channels, Feed and PlexPlayer list the stable values they already used. - The player's keydown effect is bound once on purpose (its helpers read refs): said so, with a disable, as the codebase's other deliberate sites do. - dialogFocus and the YouTube player's cleanup read refs React may already have let go of; they use what they captured when the effect ran.
This commit is contained in:
@@ -292,7 +292,7 @@ export default function Channels() {
|
|||||||
if (filtersResetToken === resetRef.current) return;
|
if (filtersResetToken === resetRef.current) return;
|
||||||
resetRef.current = filtersResetToken;
|
resetRef.current = filtersResetToken;
|
||||||
setSearch("");
|
setSearch("");
|
||||||
}, [filtersResetToken]);
|
}, [filtersResetToken, setSearch]);
|
||||||
|
|
||||||
// Tag/priority/text filters stay controlled COLUMN filters (`filters.tags` etc.) shared by the top
|
// Tag/priority/text filters stay controlled COLUMN filters (`filters.tags` etc.) shared by the top
|
||||||
// chips, the column funnels and the cards. The sync-status filter (`filters.sync`) is no longer a
|
// chips, the column funnels and the cards. The sync-status filter (`filters.sync`) is no longer a
|
||||||
|
|||||||
@@ -230,15 +230,15 @@ export default function Feed({
|
|||||||
// rows are unchanged, so clearing overrides there would make a just-hidden card flash back.
|
// rows are unchanged, so clearing overrides there would make a just-hidden card flash back.
|
||||||
// Only a real refetch (page count doesn't grow) is authoritative.
|
// Only a real refetch (page count doesn't grow) is authoritative.
|
||||||
const pagesLenRef = useRef(0);
|
const pagesLenRef = useRef(0);
|
||||||
|
const pagesLen = query.data?.pages?.length ?? 0;
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const len = query.data?.pages?.length ?? 0;
|
const appended = pagesLen > pagesLenRef.current;
|
||||||
const appended = len > pagesLenRef.current;
|
pagesLenRef.current = pagesLen;
|
||||||
pagesLenRef.current = len;
|
|
||||||
if (!appended) {
|
if (!appended) {
|
||||||
setOverrides({});
|
setOverrides({});
|
||||||
setSavedOverrides({});
|
setSavedOverrides({});
|
||||||
}
|
}
|
||||||
}, [query.dataUpdatedAt]);
|
}, [query.dataUpdatedAt, pagesLen]);
|
||||||
|
|
||||||
// One shared query with the sidebar (same key + filters -> React Query dedupes to a single request):
|
// One shared query with the sidebar (same key + filters -> React Query dedupes to a single request):
|
||||||
// the feed reads `.count`, the sidebar reads `.facets` (PERF-13, replaces feedCount + facets).
|
// the feed reads `.count`, the sidebar reads `.facets` (PERF-13, replaces feedCount + facets).
|
||||||
|
|||||||
@@ -489,6 +489,10 @@ export default function PlayerModal({
|
|||||||
window.removeEventListener("keydown", onKey);
|
window.removeEventListener("keydown", onKey);
|
||||||
// (The debounced volume write is flushed by useYtVolume's own unmount cleanup.)
|
// (The debounced volume write is flushed by useYtVolume's own unmount cleanup.)
|
||||||
};
|
};
|
||||||
|
// Bound once per player on purpose: every helper the handler calls reads its live state through
|
||||||
|
// a ref (playerRef, navRef, maximizedRef, isFullscreenRef, the volume hook's own ref), so
|
||||||
|
// re-binding on each render would only churn the listener, never change what a key does.
|
||||||
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [onClose]);
|
}, [onClose]);
|
||||||
|
|
||||||
// Create the player, resume from the saved position, checkpoint progress, and auto-mark watched at
|
// Create the player, resume from the saved position, checkpoint progress, and auto-mark watched at
|
||||||
|
|||||||
@@ -227,12 +227,13 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
|
|||||||
// Must be stable: an inline arrow would detach (null) and re-attach the node on every render,
|
// Must be stable: an inline arrow would detach (null) and re-attach the node on every render,
|
||||||
// and the fade's node-state ref would re-render in response — a loop. `fade.ref` is a setState.
|
// and the fade's node-state ref would re-render in response — a loop. `fade.ref` is a setState.
|
||||||
const tracksFade = useScrollFade();
|
const tracksFade = useScrollFade();
|
||||||
|
const tracksFadeRef = tracksFade.ref;
|
||||||
const setTracksNode = useCallback(
|
const setTracksNode = useCallback(
|
||||||
(node: HTMLElement | null) => {
|
(node: HTMLElement | null) => {
|
||||||
tracksRef.current = node as HTMLDivElement | null;
|
tracksRef.current = node as HTMLDivElement | null;
|
||||||
tracksFade.ref(node);
|
tracksFadeRef(node);
|
||||||
},
|
},
|
||||||
[tracksFade.ref],
|
[tracksFadeRef],
|
||||||
);
|
);
|
||||||
// The keyboard cheat sheet is app-wide now (ui/KeyboardHelp, opened by the global "h"), not this
|
// The keyboard cheat sheet is app-wide now (ui/KeyboardHelp, opened by the global "h"), not this
|
||||||
// player's own overlay. Mirror its open state into a ref because usePlayerVolume still needs to
|
// player's own overlay. Mirror its open state into a ref because usePlayerVolume still needs to
|
||||||
|
|||||||
@@ -10,19 +10,23 @@ import { notify } from "../lib/notifications";
|
|||||||
import { notifyActionError } from "../lib/actionErrors";
|
import { notifyActionError } from "../lib/actionErrors";
|
||||||
import { copyText } from "../lib/clipboard";
|
import { copyText } from "../lib/clipboard";
|
||||||
import { formatDate } from "../lib/format";
|
import { formatDate } from "../lib/format";
|
||||||
import { api, type DownloadJob, type ShareLink } from "../lib/api";
|
import { api, type DownloadJob, type ShareLink, type ShareRecipient } from "../lib/api";
|
||||||
import { inputCls } from "./ui/form";
|
import { inputCls } from "./ui/form";
|
||||||
import { Button } from "./ui/Button";
|
import { Button } from "./ui/Button";
|
||||||
const EXPIRY_DAYS = [null, 1, 7, 30] as const;
|
const EXPIRY_DAYS = [null, 1, 7, 30] as const;
|
||||||
|
|
||||||
// --- share with a registered user (ACL) -----------------------------------------------------
|
// --- share with a registered user (ACL) -----------------------------------------------------
|
||||||
|
|
||||||
|
// One shared empty list while the recipients load: a fresh `[]` per render made `filtered` below
|
||||||
|
// recompute on every render.
|
||||||
|
const NO_RECIPIENTS: ShareRecipient[] = [];
|
||||||
|
|
||||||
function UserShare({ job }: { job: DownloadJob }) {
|
function UserShare({ job }: { job: DownloadJob }) {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
const [q, setQ] = useState("");
|
const [q, setQ] = useState("");
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
const recipients = useQuery({ queryKey: qk.shareRecipients(), queryFn: api.shareRecipients });
|
const recipients = useQuery({ queryKey: qk.shareRecipients(), queryFn: api.shareRecipients });
|
||||||
const list = recipients.data ?? [];
|
const list = recipients.data ?? NO_RECIPIENTS;
|
||||||
const filtered = useMemo(() => {
|
const filtered = useMemo(() => {
|
||||||
const s = q.trim().toLowerCase();
|
const s = q.trim().toLowerCase();
|
||||||
if (!s) return list.slice(0, 8);
|
if (!s) return list.slice(0, 8);
|
||||||
|
|||||||
@@ -130,10 +130,13 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos
|
|||||||
const v = videoRef.current;
|
const v = videoRef.current;
|
||||||
if (v) setCurrent(v.currentTime);
|
if (v) setCurrent(v.currentTime);
|
||||||
};
|
};
|
||||||
const seek = (s: number) => {
|
const seek = useCallback(
|
||||||
const v = videoRef.current;
|
(s: number) => {
|
||||||
if (v) v.currentTime = Math.max(0, Math.min(duration, s));
|
const v = videoRef.current;
|
||||||
};
|
if (v) v.currentTime = Math.max(0, Math.min(duration, s));
|
||||||
|
},
|
||||||
|
[duration],
|
||||||
|
);
|
||||||
const playPause = () => {
|
const playPause = () => {
|
||||||
const v = videoRef.current;
|
const v = videoRef.current;
|
||||||
if (!v) return;
|
if (!v) return;
|
||||||
@@ -154,18 +157,21 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos
|
|||||||
return next;
|
return next;
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
const moveBoundary = (bi: number, time: number) =>
|
const moveBoundary = useCallback(
|
||||||
setSegments((segs) => {
|
(bi: number, time: number) =>
|
||||||
if (creating.current || bi < 0 || bi >= segs.length - 1) return segs;
|
setSegments((segs) => {
|
||||||
// bi is in [0, length-2] here, so bi and bi+1 both index existing segments.
|
if (creating.current || bi < 0 || bi >= segs.length - 1) return segs;
|
||||||
const lo = segs[bi]!.start + MIN_SEG;
|
// bi is in [0, length-2] here, so bi and bi+1 both index existing segments.
|
||||||
const hi = segs[bi + 1]!.end - MIN_SEG;
|
const lo = segs[bi]!.start + MIN_SEG;
|
||||||
const tt = Math.max(lo, Math.min(hi, time));
|
const hi = segs[bi + 1]!.end - MIN_SEG;
|
||||||
const copy = [...segs];
|
const tt = Math.max(lo, Math.min(hi, time));
|
||||||
copy[bi] = { ...copy[bi]!, end: tt };
|
const copy = [...segs];
|
||||||
copy[bi + 1] = { ...copy[bi + 1]!, start: tt };
|
copy[bi] = { ...copy[bi]!, end: tt };
|
||||||
return copy;
|
copy[bi + 1] = { ...copy[bi + 1]!, start: tt };
|
||||||
});
|
return copy;
|
||||||
|
}),
|
||||||
|
[],
|
||||||
|
);
|
||||||
const deleteBoundary = (bi: number) =>
|
const deleteBoundary = (bi: number) =>
|
||||||
setSegments((segs) => {
|
setSegments((segs) => {
|
||||||
if (creating.current || segs.length < 2) return segs;
|
if (creating.current || segs.length < 2) return segs;
|
||||||
@@ -205,7 +211,7 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos
|
|||||||
window.removeEventListener("pointermove", move);
|
window.removeEventListener("pointermove", move);
|
||||||
window.removeEventListener("pointerup", up);
|
window.removeEventListener("pointerup", up);
|
||||||
};
|
};
|
||||||
}, [drag, timeAt]);
|
}, [drag, timeAt, seek, moveBoundary]);
|
||||||
|
|
||||||
// --- crop drag ---
|
// --- crop drag ---
|
||||||
const overlayRef = useRef<HTMLDivElement>(null);
|
const overlayRef = useRef<HTMLDivElement>(null);
|
||||||
|
|||||||
@@ -51,9 +51,19 @@ export function WizardProvider({ children }: { children: ReactNode }) {
|
|||||||
|
|
||||||
// First-login onboarding: prompt the user to connect YouTube (and resume the flow after each
|
// First-login onboarding: prompt the user to connect YouTube (and resume the flow after each
|
||||||
// consent redirect). Re-checked when the account or its grants change.
|
// consent redirect). Re-checked when the account or its grants change.
|
||||||
|
// Keyed on the fields that matter, NOT the `me` object: the 60 s refetch hands out a new object,
|
||||||
|
// and re-running on that would reopen a wizard the user just closed. `can_write` is read by nothing
|
||||||
|
// here but is a trigger: a completed write grant resumes the flow.
|
||||||
|
const meId = me?.id;
|
||||||
|
const canRead = me?.can_read;
|
||||||
|
const canWrite = me?.can_write;
|
||||||
|
const isDemo = me?.is_demo;
|
||||||
|
const googleEnabled = me?.google_enabled;
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (me && shouldAutoOpenOnboarding(me)) setOpen(true);
|
if (meId == null || canRead == null) return;
|
||||||
}, [me?.id, me?.can_read, me?.can_write]);
|
const account = { can_read: canRead, is_demo: isDemo, google_enabled: googleEnabled };
|
||||||
|
if (shouldAutoOpenOnboarding(account)) setOpen(true);
|
||||||
|
}, [meId, canRead, canWrite, isDemo, googleEnabled]);
|
||||||
|
|
||||||
const actions = useMemo<WizardActions>(
|
const actions = useMemo<WizardActions>(
|
||||||
() => ({ openWizard, closeWizard }),
|
() => ({ openWizard, closeWizard }),
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next";
|
|||||||
import { keepPreviousData, useQuery } from "@tanstack/react-query";
|
import { keepPreviousData, useQuery } from "@tanstack/react-query";
|
||||||
import { ChevronDown } from "lucide-react";
|
import { ChevronDown } from "lucide-react";
|
||||||
import { qk } from "../lib/queryKeys";
|
import { qk } from "../lib/queryKeys";
|
||||||
import { api, type FeedFilters } from "../lib/api";
|
import { api, type FeedFilters, type YearCount } from "../lib/api";
|
||||||
import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./ui/AnchoredPopover";
|
import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./ui/AnchoredPopover";
|
||||||
import { filterChipClass } from "./ui/FilterChipGroup";
|
import { filterChipClass } from "./ui/FilterChipGroup";
|
||||||
import CountBadge from "./ui/CountBadge";
|
import CountBadge from "./ui/CountBadge";
|
||||||
@@ -21,6 +21,8 @@ import { useDebounced } from "../lib/useDebounced";
|
|||||||
// own category.
|
// own category.
|
||||||
|
|
||||||
const QUICK = 6; // recent-year chips shown before the "More…" overflow
|
const QUICK = 6; // recent-year chips shown before the "More…" overflow
|
||||||
|
// One shared empty list while the counts load: a fresh `[]` per render made the memos below recompute.
|
||||||
|
const NO_YEARS: YearCount[] = [];
|
||||||
|
|
||||||
export default function YearFilter({
|
export default function YearFilter({
|
||||||
filters,
|
filters,
|
||||||
@@ -51,7 +53,7 @@ export default function YearFilter({
|
|||||||
staleTime: 30_000,
|
staleTime: 30_000,
|
||||||
placeholderData: keepPreviousData,
|
placeholderData: keepPreviousData,
|
||||||
});
|
});
|
||||||
const years = yearsQuery.data?.years ?? [];
|
const years = yearsQuery.data?.years ?? NO_YEARS;
|
||||||
const selected = filters.year;
|
const selected = filters.year;
|
||||||
|
|
||||||
// A year pick and the Upload-date control are mutually exclusive publish-date narrowings.
|
// A year pick and the Upload-date control are mutually exclusive publish-date narrowings.
|
||||||
|
|||||||
@@ -108,6 +108,11 @@ export function useDialogChrome(
|
|||||||
// the scope itself, with `focusScope`), so the dialog opens focused rather than stranded on the
|
// the scope itself, with `focusScope`), so the dialog opens focused rather than stranded on the
|
||||||
// background.
|
// background.
|
||||||
const scope = scopeRef.current;
|
const scope = scopeRef.current;
|
||||||
|
// The dialog's element as it is while open, for the restore in the CLEANUP — where a closing
|
||||||
|
// dialog's refs are already detached (refs detach in the commit, passive cleanups run after).
|
||||||
|
// Today that reads the same either way (a detached owner and a null one both fail every
|
||||||
|
// `contains` below), but the cleanup should not depend on a ref React has already let go of.
|
||||||
|
const ownerAtOpen = ownerRef?.current ?? scope;
|
||||||
if (scope && !scope.contains(document.activeElement)) {
|
if (scope && !scope.contains(document.activeElement)) {
|
||||||
const back = focusedRef.current;
|
const back = focusedRef.current;
|
||||||
const target =
|
const target =
|
||||||
@@ -151,7 +156,7 @@ export function useDialogChrome(
|
|||||||
// this, a dialog closing on its own (an in-flight mutation's `onSuccess`) under a modal raised
|
// this, a dialog closing on its own (an in-flight mutation's `onSuccess`) under a modal raised
|
||||||
// above it pulled the focus out of that standing dialog onto our own trigger, behind its
|
// above it pulled the focus out of that standing dialog onto our own trigger, behind its
|
||||||
// scrim — where Enter still activated it (deep-review round 7).
|
// scrim — where Enter still activated it (deep-review round 7).
|
||||||
const owner = ownerRef?.current ?? scopeRef.current;
|
const owner = ownerAtOpen;
|
||||||
const active = document.activeElement;
|
const active = document.activeElement;
|
||||||
const above = active instanceof Element ? active.closest('[aria-modal="true"]') : null;
|
const above = active instanceof Element ? active.closest('[aria-modal="true"]') : null;
|
||||||
const standingAbove =
|
const standingAbove =
|
||||||
|
|||||||
@@ -441,6 +441,8 @@ export function useYtProgress({
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The session's positions map (mutated in place, never replaced) — held here for the cleanup.
|
||||||
|
const sessionPos = sessionPosRef.current;
|
||||||
// Periodic checkpoint so progress (and auto-watch) survive a crash/refresh.
|
// Periodic checkpoint so progress (and auto-watch) survive a crash/refresh.
|
||||||
const timer = window.setInterval(persist, 5000);
|
const timer = window.setInterval(persist, 5000);
|
||||||
// Save on page unload too (F5/close/navigate): effect cleanup doesn't run on a full reload. A
|
// Save on page unload too (F5/close/navigate): effect cleanup doesn't run on a full reload. A
|
||||||
@@ -469,7 +471,7 @@ export function useYtProgress({
|
|||||||
// not enough on its own: nothing refetches the feed until it remounts (staleTime 30s,
|
// not enough on its own: nothing refetches the feed until it remounts (staleTime 30s,
|
||||||
// refetchOnWindowFocus off), so a reopen used to resume from the position the cache was
|
// refetchOnWindowFocus off), so a reopen used to resume from the position the cache was
|
||||||
// fetched with — and the next checkpoint then persisted that older value over this one.
|
// fetched with — and the next checkpoint then persisted that older value over this one.
|
||||||
patchCachedProgress(qc, sessionPosRef.current);
|
patchCachedProgress(qc, sessionPos);
|
||||||
qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
|
qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
|
||||||
qc.invalidateQueries({ queryKey: qk.playlist(), refetchType: "none" });
|
qc.invalidateQueries({ queryKey: qk.playlist(), refetchType: "none" });
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user