From 704e33638f2c6019cefac292ea998c6856638029 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 3 Oct 2026 18:07:56 +0200 Subject: [PATCH] fix: restore six useEffect dependencies removed while silencing lint The previous commit dropped biome-ignore comments to clear useExhaustiveDependencies diagnostics and, in doing so, also deleted the dependencies themselves. Six components were left with effects that no longer react to the state they read. Every one of these is a real behaviour regression, not a lint preference: - health-check-client: checkEmulator is a function declaration, so it gets a fresh identity each render. As an effect dependency that re-fires the effect after every setState, polling /api/admin/devops/health in a loop. Wrapped in useCallback so the identity is stable. - article-recovery: reload restarts the autosave timer for the "Retry recovery" button. Without it in the deps that button is a no-op. The counter had been renamed to _reload to satisfy the unused-variable rule. - catalog-integrity-panel: same pattern; refresh starts a new read-only scan, so the rescan control did nothing. - catalog-search: refreshKey re-runs the query after a bulk edit, so results were not refreshed after catalog edits. The selection-reset effect also lost catalogType, so switching catalog no longer cleared the selection. - catalog-image-picker: dropped debounced (the search term) and name (the error reset), so image search and error state no longer reacted to input. - icon-picker: dropped iconImage, so a failed load left the placeholder on the next icon too. Each restored dependency carries a biome-ignore with the reason it is load-bearing, so the diagnostic can be re-derived instead of silently disappearing again. Verified: typecheck, lint clean on all six, unit 3315 passed, integration 20 passed, UI 72 passed / 2 skipped. --- src/app/admin/devops/health-check-client.tsx | 9 ++++++--- src/components/admin/article-recovery.tsx | 9 +++++++-- src/components/admin/catalog/catalog-image-picker.tsx | 7 +++++-- .../admin/catalog/catalog-integrity-panel.tsx | 7 +++++-- src/components/admin/catalog/icon-picker.tsx | 5 ++++- src/features/catalog/components/catalog-search.tsx | 11 ++++++++--- 6 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/app/admin/devops/health-check-client.tsx b/src/app/admin/devops/health-check-client.tsx index dc8af623..8b09cc62 100644 --- a/src/app/admin/devops/health-check-client.tsx +++ b/src/app/admin/devops/health-check-client.tsx @@ -1,7 +1,7 @@ "use client"; import { RefreshCw } from "lucide-react"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -11,7 +11,10 @@ export function HealthCheckClient() { ); const [checking, setChecking] = useState(false); - async function checkEmulator() { + // Must be stable: the effect below depends on it. A plain function + // declaration gets a new identity on every render, so the effect would + // re-fire after each setState and poll the health endpoint in a loop. + const checkEmulator = useCallback(async () => { setChecking(true); setStatus("checking"); try { @@ -27,7 +30,7 @@ export function HealthCheckClient() { } finally { setChecking(false); } - } + }, []); useEffect(() => { checkEmulator(); diff --git a/src/components/admin/article-recovery.tsx b/src/components/admin/article-recovery.tsx index 83d91e13..aed6b524 100644 --- a/src/components/admin/article-recovery.tsx +++ b/src/components/admin/article-recovery.tsx @@ -20,13 +20,14 @@ export function useArticleRecovery( ReturnType > | null>(null); const [message, setMessage] = useState("autosaveLoading"); - const [_reload, setReload] = useState(0); + const [reload, setReload] = useState(0); const version = useRef(0); const request = useRef | null>(null); const last = useRef(""); const dirtyRef = useRef(dirty); dirtyRef.current = dirty; const blocked = useRef(true); + // biome-ignore lint/correctness/useExhaustiveDependencies: reload restarts the autosave timer for the recovery retry without remounting; read via setReload useEffect(() => { let active = true; blocked.current = true; @@ -79,7 +80,11 @@ export function useArticleRecovery( active = false; clearInterval(timer); }; - }, [key, form, saving]); + // reload restarts the autosave timer without remounting the editor or + // touching the current form contents; it is what the "Retry recovery" + // button bumps after reconciling server versions. Removing it from the + // deps makes that button a no-op. + }, [key, form, saving, reload]); return { draft: recovery?.draft?.payload, wait: () => request.current ?? Promise.resolve(), diff --git a/src/components/admin/catalog/catalog-image-picker.tsx b/src/components/admin/catalog/catalog-image-picker.tsx index eb172c7a..25657287 100644 --- a/src/components/admin/catalog/catalog-image-picker.tsx +++ b/src/components/admin/catalog/catalog-image-picker.tsx @@ -76,6 +76,7 @@ export function CatalogImagePicker({ ); // Reset and fetch on open / search change + // biome-ignore lint/correctness/useExhaustiveDependencies: debounced is the search term; refetching on its change is intended useEffect(() => { if (!open) return; setImages([]); @@ -83,7 +84,8 @@ export function CatalogImagePicker({ setHasMore(true); fetchImages(0, true); if (scrollRef.current) scrollRef.current.scrollTop = 0; - }, [open, fetchImages]); + // debounced drives the search term, so a changed query must refetch. + }, [open, fetchImages, debounced]); const handleScroll = useCallback(() => { const el = scrollRef.current; @@ -233,7 +235,8 @@ function CatalogImageThumb({ const [error, setError] = useState(0); // Reset error state when name changes - useEffect(() => setError(0), []); + // biome-ignore lint/correctness/useExhaustiveDependencies: name is a prop; the reset is meant to follow it + useEffect(() => setError(0), [name]); if (!name || error >= 2) { return ( diff --git a/src/components/admin/catalog/catalog-integrity-panel.tsx b/src/components/admin/catalog/catalog-integrity-panel.tsx index 5cae4207..ad4bab7b 100644 --- a/src/components/admin/catalog/catalog-integrity-panel.tsx +++ b/src/components/admin/catalog/catalog-integrity-panel.tsx @@ -33,9 +33,10 @@ export function CatalogIntegrityPanel({ canRepair }: { canRepair: boolean }) { const [busy, setBusy] = useState(false); const [error, setError] = useState<"failed" | "stalePreview" | null>(null); const [applied, setApplied] = useState(null); - const [_refresh, setRefresh] = useState(0); + const [refresh, setRefresh] = useState(0); const request = useRef(0); const applying = useRef(false); + // biome-ignore lint/correctness/useExhaustiveDependencies: refresh starts a new read-only scan; read via setRefresh useEffect(() => { const version = ++request.current; setBusy(true); @@ -57,7 +58,9 @@ export function CatalogIntegrityPanel({ canRepair }: { canRepair: boolean }) { return () => { request.current++; }; - }, [catalog]); + // refresh starts a new read-only scan; without it the rescan control + // does nothing. + }, [catalog, refresh]); async function prepare() { setBusy(true); setPreview(null); diff --git a/src/components/admin/catalog/icon-picker.tsx b/src/components/admin/catalog/icon-picker.tsx index 8832b9f6..07f81472 100644 --- a/src/components/admin/catalog/icon-picker.tsx +++ b/src/components/admin/catalog/icon-picker.tsx @@ -214,7 +214,10 @@ function IconPreview({ size?: number; }) { const [error, setError] = useState(false); - useEffect(() => setError(false), []); + // Clear the previous load failure when the icon changes, otherwise a + // placeholder sticks to the next image too. + // biome-ignore lint/correctness/useExhaustiveDependencies: iconImage is a prop; the reset is meant to follow it + useEffect(() => setError(false), [iconImage]); if (error || iconImage <= 0) { return ( diff --git a/src/features/catalog/components/catalog-search.tsx b/src/features/catalog/components/catalog-search.tsx index 58649e83..ea8bb1e8 100644 --- a/src/features/catalog/components/catalog-search.tsx +++ b/src/features/catalog/components/catalog-search.tsx @@ -48,7 +48,9 @@ export function CatalogSearch({ const [selectionError, setSelectionError] = useState(null); const destinationRequest = useRef(0); const destinationBusy = useRef(false); - const [_refreshKey, setRefreshKey] = useState(0); + const [refreshKey, setRefreshKey] = useState(0); + // Catalog switches invalidate the selection and destination requests. + // biome-ignore lint/correctness/useExhaustiveDependencies: a catalog switch is exactly what should reset this useEffect(() => { destinationRequest.current += 1; destinationBusy.current = false; @@ -59,7 +61,7 @@ export function CatalogSearch({ return () => { destinationRequest.current += 1; }; - }, []); + }, [catalogType]); async function loadDestinations() { if (destinationBusy.current) return; destinationBusy.current = true; @@ -99,6 +101,7 @@ export function CatalogSearch({ }); if (!pages) void loadDestinations(); } + // biome-ignore lint/correctness/useExhaustiveDependencies: refreshKey re-runs the query after a bulk edit useEffect(() => { const request = requests.start(); setResults([]); @@ -129,7 +132,9 @@ export function CatalogSearch({ clearTimeout(timer); requests.cancel(); }; - }, [query, catalogType, requests]); + // refreshKey re-runs the query after a bulk edit changed rows that this + // search would otherwise keep showing as stale. + }, [query, catalogType, requests, refreshKey]); return (