From ec84face762d06f02f28495c941eb32aa96ba1b8 Mon Sep 17 00:00:00 2001 From: ci Date: Tue, 28 Jul 2026 00:37:56 +0100 Subject: [PATCH] fix(hooks): clear the last react-hooks warnings, promote the 5 rules to error (#38 batch 3/3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes out the #38 burn-down (28 warnings -> 0): - admin/appearance: the restore-theme-on-leave cleanup needs the latest saved settings + palette override but must run only on unmount, so those were mirrored into refs *during render* (react-hooks/refs). Now mirrored from an effect (refs belong outside render); unmount cleanup reads them unchanged. - route-progress: reordered "finish" above "start" so it's a real dependency instead of used-before-declaration behind a lint-disable — clearing the immutability + preserve-manual-memoization warnings and dropping two eslint-disable comments. The route-commit effect now finishes on the next animation frame rather than synchronously (a genuine react-to-external-event; deferring also keeps the 100%-then-fade paint out of the route's own render). With the count at zero, the five React-Compiler-era rules (refs, set-state-in-effect, static-components, immutability, preserve-manual-memoization) are promoted from warn to error - explicit, so CI blocks any regression and the codebase stays React-Compiler-ready. Verified live: appearance palette preview applies and, on leaving unsaved, restores the saved palette (Sandstone preview -> Harbor on nav-away); the route progress bar shows during navigation and hides on commit. tsc clean, eslint 0 problems, vitest 130/130. Co-Authored-By: Claude --- frontend/eslint.config.mjs | 20 ++++++------- .../src/app/(app)/admin/appearance/page.tsx | 11 +++++-- .../src/components/app/route-progress.tsx | 30 +++++++++++-------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/frontend/eslint.config.mjs b/frontend/eslint.config.mjs index ef9679a..bb4d9ca 100644 --- a/frontend/eslint.config.mjs +++ b/frontend/eslint.config.mjs @@ -25,17 +25,15 @@ const eslintConfig = [ "error", { argsIgnorePattern: "^_", varsIgnorePattern: "^_" }, ], - // react-hooks v7 (bundled by eslint-config-next 16) adds the React - // Compiler-era rules as errors. The codebase predates them and carries - // ~27 hits of deliberate pre-compiler idioms (latest-ref, sync-state-in - // -effect, render-scoped subcomponents). Warn — visible in editors and - // lint output, not a CI gate — while they're burned down incrementally. - // rules-of-hooks / exhaustive-deps stay errors, as ever. - "react-hooks/refs": "warn", - "react-hooks/set-state-in-effect": "warn", - "react-hooks/static-components": "warn", - "react-hooks/immutability": "warn", - "react-hooks/preserve-manual-memoization": "warn", + // react-hooks v7 (eslint-config-next 16) React Compiler-era rules, held at + // error so a new pre-compiler idiom can't slip in (the codebase's original + // hits were burned down in #38). Explicit rather than inherited so the CI + // gate is unambiguous. + "react-hooks/refs": "error", + "react-hooks/set-state-in-effect": "error", + "react-hooks/static-components": "error", + "react-hooks/immutability": "error", + "react-hooks/preserve-manual-memoization": "error", }, }, diff --git a/frontend/src/app/(app)/admin/appearance/page.tsx b/frontend/src/app/(app)/admin/appearance/page.tsx index 21a24fa..287cb28 100644 --- a/frontend/src/app/(app)/admin/appearance/page.tsx +++ b/frontend/src/app/(app)/admin/appearance/page.tsx @@ -64,11 +64,16 @@ export default function AdminAppearancePage() { }, [palette, accent]); // On leaving without saving, restore what the viewer actually sees (their own - // palette override, if any, over the saved site default). + // palette override, if any, over the saved site default). The unmount cleanup + // needs the *latest* saved settings + override, but must run only on unmount — + // so mirror them into refs from an effect (refs are written outside render) + // and read those in the mount-only cleanup. const savedRef = useRef(saved); - savedRef.current = saved; const overrideRef = useRef(paletteOverride); - overrideRef.current = paletteOverride; + useEffect(() => { + savedRef.current = saved; + overrideRef.current = paletteOverride; + }); useEffect(() => { return () => { const s = savedRef.current; diff --git a/frontend/src/components/app/route-progress.tsx b/frontend/src/components/app/route-progress.tsx index 35ad4a9..a6d58f4 100644 --- a/frontend/src/components/app/route-progress.tsx +++ b/frontend/src/components/app/route-progress.tsx @@ -24,6 +24,15 @@ export function RouteProgress() { } }, []); + // Declared before `start` so it can be a real dependency there (rather than + // referenced before declaration behind a lint-disable). + const finish = React.useCallback(() => { + clearTimers(); + // If the bar never showed (instant nav within the start delay), stay hidden. + setProgress((p) => (p <= 0 ? 0 : 100)); + timers.current.push(setTimeout(() => setProgress(0), 240)); + }, [clearTimers]); + const start = React.useCallback(() => { clearTimers(); // Delay before showing so a fast navigation doesn't blink a bar. @@ -39,15 +48,7 @@ export function RouteProgress() { ); // Never trickle forever if a navigation is cancelled. timers.current.push(setTimeout(() => finish(), 10_000)); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [clearTimers]); - - const finish = React.useCallback(() => { - clearTimers(); - // If the bar never showed (instant nav within the start delay), stay hidden. - setProgress((p) => (p <= 0 ? 0 : 100)); - timers.current.push(setTimeout(() => setProgress(0), 240)); - }, [clearTimers]); + }, [clearTimers, finish]); // Start on any left-click of an internal, same-origin link (Next renders // a plain ). Modified clicks and new-tab links open elsewhere — ignore them. @@ -81,11 +82,14 @@ export function RouteProgress() { return () => document.removeEventListener("click", onClick, true); }, [pathname, start]); - // A committed pathname change means the navigation finished. + // A committed pathname change means the navigation finished. Finish on the + // next frame rather than synchronously in the effect body: it's a genuine + // react-to-external-event (the router committed), and deferring keeps the + // 100%-then-fade paint from being coalesced into the route's own render. React.useEffect(() => { - finish(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [pathname]); + const raf = requestAnimationFrame(() => finish()); + return () => cancelAnimationFrame(raf); + }, [pathname, finish]); React.useEffect(() => clearTimers, [clearTimers]);