refactor(frontend): the context providers adjust state in render, not after it (E27 S2)

Eleven suppressions across the five providers. The recurring shape here was "an effect that
undoes something when you navigate away", and every one of them ran a commit LATE — the new
page painted once while the old module's state was still live.

Adjusted during render instead (React discards that pass rather than committing it, so there
is no in-between frame):
- SavedViewsProvider reset the sticky view and closed its modal from an effect on `source`, so
  the first Plex render carried the feed's sticky id — long enough for a stale view chip. It
  also followed the active view into stickyId from a second effect; that one converges by
  construction, since preferSticky returns the sticky match when there is one.
- NavigationProvider ended a live YouTube search from an effect on `page`. Keyed on the page
  CHANGE rather than derived from it, because "ended" has to be permanent — deriving would have
  quietly brought the search back on return to the feed.
- ChannelsProvider's manager focus is now simply DERIVED from the page: it means nothing off
  the Channels page, and focusChannel sets it and navigates in one handler.

Honest deps by freezing what is genuinely mount-time:
- FeedFiltersProvider persisted a share link's filters with an empty dep list; listing `filters`
  honestly would have re-persisted on every edit and spun. The mount value is frozen into its
  own state, so all three deps are stable and the effect says what it does.
- ChannelsProvider's URL-seed layout effect and NavigationProvider's initial history stamp get
  the same treatment. The three slice setters they call were already stable useCallbacks
  (useFilterState / useSortState / useAccountPersistedState all return one).
- SavedViewsProvider reads `cfg` through a ref: it re-memoizes on every filter edit, and the
  per-source seed guard already enforces once. That also made the set-state finding go away —
  with the render value gone, the checker can see the seed for what it is.

WizardProvider keeps its one escape hatch, now inline with the invariant: auto-opening is a
one-way decision taken from server data as it arrives, and deriving it would reopen a wizard
the user had just closed.

Suppressions 89 -> 78; exhaustive-deps 27 -> 23.
This commit is contained in:
2026-09-21 00:12:44 +02:00
parent 6a7ff330f6
commit ea6d829c29
6 changed files with 59 additions and 50 deletions
-20
View File
@@ -14,11 +14,6 @@
"count": 2
}
},
"src/components/ChannelsProvider.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
}
},
"src/components/ConfigPanel.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
@@ -85,11 +80,6 @@
"count": 1
}
},
"src/components/NavigationProvider.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
}
},
"src/components/PersonalViewsModal.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
@@ -134,11 +124,6 @@
"count": 1
}
},
"src/components/SavedViewsProvider.tsx": {
"react-hooks/set-state-in-effect": {
"count": 3
}
},
"src/components/Scheduler.tsx": {
"react-hooks/set-state-in-effect": {
"count": 2
@@ -164,11 +149,6 @@
"count": 1
}
},
"src/components/WizardProvider.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
}
},
"src/components/comfortPin.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
+13 -8
View File
@@ -110,7 +110,7 @@ export function ChannelsProvider({ children }: { children: ReactNode }) {
// it against the live columns for display, so the provider stores it un-healed (no options passed).
const [sort, setSort] = useSortState(LS.channelSort);
const [filtersResetToken, setFiltersResetToken] = useState(0);
const [focusName, setFocusName] = useState<string | null>(null);
const [focusNameRaw, setFocusName] = useState<string | null>(null);
const [focusToken, setFocusToken] = useState(0);
// S8 — shareable manager view. Captured once at mount (or null when no manager URL keys are present).
@@ -120,14 +120,18 @@ export function ChannelsProvider({ children }: { children: ReactNode }) {
// Apply the shared-link seed once, BEFORE paint, through the slice setters — which persist it, so the
// shared view also survives a later PLAIN reload (no URL). setViewRaw (not setView) restores the tab
// without pushing a phantom history entry. A normal load has urlSeed === null and this is a no-op.
// `view` is frozen at mount for the same reason `urlSeed` is: the comparison is against the tab
// this load started on, and the three setters are stable by construction (useFilterState,
// useSortState and useAccountPersistedState all return a useCallback). So the dep list can name
// every value the body reads without the seed re-arming on a later tab switch.
const [mountView] = useState(view);
useLayoutEffect(() => {
if (!urlSeed || seededRef.current) return;
seededRef.current = true;
if (urlSeed.view !== view) setViewRaw(urlSeed.view);
if (urlSeed.view !== mountView) setViewRaw(urlSeed.view);
setFilters(urlSeed.filters);
setSort(urlSeed.sort);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [urlSeed, mountView, setViewRaw, setFilters, setSort]);
// Latest-value ref so the STABLE setView can compare against the current tab without depending on
// it (a dep would recreate the action on every switch and re-render every actions consumer).
@@ -175,10 +179,11 @@ export function ChannelsProvider({ children }: { children: ReactNode }) {
setPage("channels");
}, [setFilters, setView, setPage]);
// Clear the manager focus when leaving the Channels page so it doesn't re-apply on a later visit.
useEffect(() => {
if (page !== "channels") setFocusName(null);
}, [page]);
// The manager focus only means anything ON the Channels page, so it is DERIVED from the page
// rather than cleared by an effect when you leave. focusChannel sets the name and navigates in the
// same handler, so the value is live by the time channels renders; everywhere else it reads null,
// one render earlier than the effect used to make it.
const focusName = page === "channels" ? focusNameRaw : null;
// Label the entry the manager was REACHED on (setPage pushes a bare { sfPage }), so stepping back
// onto it later restores the tab it belonged to rather than leaving whatever tab is current.
@@ -67,10 +67,15 @@ export function FeedFiltersProvider({ children }: { children: ReactNode }) {
// A share link's filters (captured at mount) win over the account's stored view and are persisted
// to it once, so the view the link opened also survives a later plain reload.
//
// The mount-time filters are FROZEN into their own state rather than left to an empty dep list:
// `filters` changes on every edit, so listing it honestly would re-persist — and setFilters
// setting the state it depends on would spin. Frozen, all three deps are stable and the effect
// means exactly what it says: write what the URL brought, once.
const [urlSeededFilters] = useState(filters);
useEffect(() => {
if (initialHadUrlFilters) setFilters(filters);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
if (initialHadUrlFilters) setFilters(urlSeededFilters);
}, [initialHadUrlFilters, urlSeededFilters, setFilters]);
const actions = useMemo<FeedFiltersActions>(() => ({ setFilters }), [setFilters]);
+17 -8
View File
@@ -153,10 +153,16 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
// Expose setPage to decoupled callers (download toast "View", etc.) via the lib/nav singleton.
useEffect(() => setNavigator(setPage), [setPage]);
// Leaving the feed ends any live YouTube search, so coming back shows the normal feed.
useEffect(() => {
// Leaving the feed ends any live YouTube search, so coming back shows the normal feed. Adjusted
// DURING the render that changes the page, against the page we were last on: as an effect it ran
// after the new page had already committed, so for one frame a module rendered with a live search
// still in the nav state. Keyed on the CHANGE rather than derived from `page`, because "ended" has
// to be permanent — a derived null would quietly come back when you returned to the feed.
const [pageSearchWasOn, setPageSearchWasOn] = useState(page);
if (pageSearchWasOn !== page) {
setPageSearchWasOn(page);
if (page !== "feed") setYtSearch(null);
}, [page]);
}
// Reflect the current module in the browser tab title. Feed = brand only; an open channel page
// shows the channel name. Signed out, the page on screen is the public landing whatever `page`
@@ -174,6 +180,10 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
document.title = label ? `${label} · ${brand}` : brand;
}, [signedIn, page, channelView, i18n.language, t]);
// The page this tab LOADED on. Frozen, because the stamp below belongs to the initial history
// entry: every later page change writes its own entry through setPage.
const [mountPage] = useState(page);
// In-app Back/Forward: stamp the initial entry with the current page (mirrored to the address bar),
// then sync state from history.state on popstate — the browser restores each entry's URL for us, so
// the popstate handler only needs to read history.state.
@@ -185,12 +195,12 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
// Mirror the resolved initial page to the address bar (feed implicit) so a page reached from the
// stored landing pref — not just from the URL — is itself bookmarkable. urlWithParams preserves any
// ?chan / ?v / ?pl / filter params the loaders already read.
const url = urlWithParams({ page: page === "feed" ? null : page });
const url = urlWithParams({ page: mountPage === "feed" ? null : mountPage });
if (st._yt && st._ytScrape) {
window.history.replaceState({ ...st, sfPage: page }, "", url);
window.history.replaceState({ ...st, sfPage: mountPage }, "", url);
} else {
const { _yt: _staleYt, _ytScrape: _staleScrape, ...rest } = st;
window.history.replaceState({ ...rest, sfPage: page }, "", url);
window.history.replaceState({ ...rest, sfPage: mountPage }, "", url);
}
function onPop(e: PopStateEvent) {
const p = (e.state?.sfPage as Page) ?? "feed";
@@ -207,8 +217,7 @@ export function NavigationProvider({ children }: { children: ReactNode }) {
}
window.addEventListener("popstate", onPop);
return () => window.removeEventListener("popstate", onPop);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [mountPage]);
const state = useMemo<NavigationState>(
() => ({ page, channelView, ytSearch }),
+20 -11
View File
@@ -129,11 +129,15 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
const [modalOpen, setModalOpen] = useState(false);
// Switching modules (feed ⟷ plex) starts fresh: a feed view can't be the sticky/active one on plex,
// and a feed modal shouldn't linger open on plex.
useEffect(() => {
// and a feed modal shouldn't linger open on plex. Adjusted DURING RENDER against the previous
// source rather than from an effect, so the first plex render never commits carrying the feed's
// sticky id — which is exactly long enough for a stale view chip to paint.
const [prevSource, setPrevSource] = useState(source);
if (prevSource !== source) {
setPrevSource(source);
setStickyId(null);
setModalOpen(false);
}, [source]);
}
// The ONE default-application path for both modules. On the views query's FIRST resolve for a
// source, seed the starred default view's filters — but only into a genuinely fresh module (no
@@ -146,17 +150,21 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
// per-account stored-filters key, which is always readable here: BootGate pins the tab's account
// before this provider mounts.
const seededRef = useRef<Set<ViewSource>>(new Set());
// `cfg` re-memoizes on every filter edit, so it is read through a ref: the seed must fire on the
// views query's first resolve for a source and never again, which `seededRef` enforces. Listing
// cfg would re-arm nothing (the guard still holds) but would re-run the effect on every keystroke
// in the filter bar. The ref keeps the dep list honest without that.
const cfgRef = useRef(cfg);
cfgRef.current = cfg;
useEffect(() => {
const data = viewsQuery.data;
if (!data || seededRef.current.has(source)) return;
seededRef.current.add(source);
const def = pickDefaultToSeed(data, cfg.canSeedDefault());
const c = cfgRef.current;
const def = pickDefaultToSeed(data, c.canSeedDefault());
if (!def) return;
cfg.setFilters(cfg.asFilters(def.filters));
c.setFilters(c.asFilters(def.filters));
setStickyId(def.id);
// cfg is read fresh on this resolve and the per-source guard makes the body run once, so cfg
// (which re-memoizes on every filter edit) is deliberately kept out of the deps.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [viewsQuery.data, source]);
const currentKey = cfg.keyOf(cfg.filters);
@@ -168,9 +176,10 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
),
[views, currentKey, stickyId, cfg],
);
useEffect(() => {
if (activeView) setStickyId(activeView.id);
}, [activeView]);
// The sticky id follows whichever view is active — also adjusted during render. It converges by
// construction: preferSticky returns the sticky match when there is one, so the very write below
// makes its own condition false on the next pass.
if (activeView && activeView.id !== stickyId) setStickyId(activeView.id);
const stickyView = useMemo(
() => (stickyId == null ? null : (views.find((v) => v.id === stickyId) ?? null)),
@@ -39,6 +39,7 @@ export function WizardProvider({ children }: { children: ReactNode }) {
useEffect(() => {
if (meId == null || canRead == null) return;
const account = { can_read: canRead, is_demo: isDemo, google_enabled: googleEnabled };
// eslint-disable-next-line react-hooks/set-state-in-effect -- opening is a one-way decision taken from SERVER data as it arrives, not a value derivable in render — deriving `open` from the grants would re-open a wizard the user had just closed, which is exactly what the narrowed dep list above exists to prevent.
if (shouldAutoOpenOnboarding(account)) setOpen(true);
}, [meId, canRead, canWrite, isDemo, googleEnabled]);