From 2505a5b5c9e58eb1f5f86dfeeb2b67cd67cfa2c3 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:24:15 -0300 Subject: [PATCH] fix(dashboard): read the combos usage-guide dismissal from an external store (base-red #12581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `release/v3.8.51` is red on `ESLint errors: 1 error(s)`: src/app/(dashboard)/dashboard/combos/page.tsx:774 error react-hooks/set-state-in-effect — Calling setState synchronously within an effect can trigger cascading renders The pattern was deliberate and the comment above it explains why: the dismissal lives in localStorage, SSR cannot read it, and a lazy useState initializer would hydrate with a mismatch. The effect fixed the mismatch at the cost of an extra commit of the whole page tree on every load — which is exactly what the rule (new in eslint-plugin-react-hooks 7.1.1, the version this branch pins) now rejects. `useSyncExternalStore` is the sanctioned shape for this: getServerSnapshot supplies the SSR-safe default, getSnapshot reads localStorage after hydration, and the two persistence handlers notify subscribers instead of setting state. Subscribing to `storage` keeps other tabs in sync for free. Behavior is preserved exactly, including the distinction between the two dismissals: "hide forever" persists, while plain "hide" stays per-mount and is kept as local state rather than folded into the store. Validated: the rule reproduces locally with the pinned 7.1.1 plugin (1 error) and is clean after the change; `typecheck:core` 0 errors. Note `tests/unit/ui/combos-page-smoke.test.tsx` is quarantined in vitest.config.ts — run under a non-excluded name it times out at 5000ms importing the module, identically on the unmodified base file, so that failure is pre-existing and unrelated. --- ...v3851-combos-usage-guide-external-store.md | 1 + src/app/(dashboard)/dashboard/combos/page.tsx | 71 +++++++++++++++---- 2 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 changelog.d/fixes/v3851-combos-usage-guide-external-store.md diff --git a/changelog.d/fixes/v3851-combos-usage-guide-external-store.md b/changelog.d/fixes/v3851-combos-usage-guide-external-store.md new file mode 100644 index 0000000000..ac4c882899 --- /dev/null +++ b/changelog.d/fixes/v3851-combos-usage-guide-external-store.md @@ -0,0 +1 @@ +- **fix(dashboard):** The Combos page usage guide now reads its dismissal through `useSyncExternalStore` instead of correcting SSR state inside an effect, removing an extra commit of the page tree on every load (and the `react-hooks/set-state-in-effect` error it raised). diff --git a/src/app/(dashboard)/dashboard/combos/page.tsx b/src/app/(dashboard)/dashboard/combos/page.tsx index 0105dd8723..03733ad033 100644 --- a/src/app/(dashboard)/dashboard/combos/page.tsx +++ b/src/app/(dashboard)/dashboard/combos/page.tsx @@ -1,6 +1,15 @@ "use client"; -import { useState, useEffect, useCallback, useMemo, useRef, memo, Suspense } from "react"; +import { + useState, + useEffect, + useCallback, + useMemo, + useRef, + useSyncExternalStore, + memo, + Suspense, +} from "react"; import dynamic from "next/dynamic"; import Link from "next/link"; import { useRouter, useSearchParams } from "next/navigation"; @@ -388,6 +397,42 @@ const STRATEGY_RECOMMENDATIONS_FALLBACK = { const COMBO_USAGE_GUIDE_STORAGE_KEY = "omniroute:combos:hide-usage-guide"; +// The dismissal lives in localStorage, which SSR cannot read: a lazy useState +// initializer would render "not dismissed" on the server and the real value on +// the client, and correcting that in an effect is a synchronous setState inside +// an effect (react-hooks/set-state-in-effect) that costs an extra commit of this +// whole tree. useSyncExternalStore is the sanctioned shape for exactly this — +// getServerSnapshot supplies the SSR-safe default, getSnapshot reads the store +// after hydration, and the two handlers below notify subscribers instead of +// setting state. The `storage` listener keeps other tabs in sync for free. +const usageGuideListeners = new Set<() => void>(); + +function subscribeUsageGuide(onStoreChange: () => void): () => void { + usageGuideListeners.add(onStoreChange); + globalThis.addEventListener?.("storage", onStoreChange); + return () => { + usageGuideListeners.delete(onStoreChange); + globalThis.removeEventListener?.("storage", onStoreChange); + }; +} + +function emitUsageGuideChange(): void { + for (const listener of usageGuideListeners) listener(); +} + +function getUsageGuideSnapshot(): boolean { + try { + return globalThis.localStorage?.getItem(COMBO_USAGE_GUIDE_STORAGE_KEY) !== "1"; + } catch { + // Storage access errors (privacy mode / restricted environments) show the guide. + return true; + } +} + +function getUsageGuideServerSnapshot(): boolean { + return true; +} + // Pure predicate hoisted out of the page component to keep its cyclomatic budget flat // (check:complexity new-code mode). function isStaleIntelligentSelection( @@ -768,14 +813,15 @@ function CombosPageContent() { // full client-only re-render of this tree, discarding whatever the fetch // effects below had already populated. Start with the SSR-safe default on // both passes and correct it client-only, after hydration, in an effect. - const [showUsageGuide, setShowUsageGuide] = useState(true); - useEffect(() => { - try { - setShowUsageGuide(globalThis.localStorage?.getItem(COMBO_USAGE_GUIDE_STORAGE_KEY) !== "1"); - } catch { - // Ignore storage access errors (privacy mode / restricted environments) - } - }, []); + const usageGuideNotDismissed = useSyncExternalStore( + subscribeUsageGuide, + getUsageGuideSnapshot, + getUsageGuideServerSnapshot + ); + // "Hide" (as opposed to "hide forever") is intentionally per-mount: it is not + // persisted, and remounting the page brings the guide back — same as before. + const [usageGuideHiddenForNow, setUsageGuideHiddenForNow] = useState(false); + const showUsageGuide = usageGuideNotDismissed && !usageGuideHiddenForNow; const [recentlyCreatedCombo, setRecentlyCreatedCombo] = useState(""); const [creatingKimiPreset, setCreatingKimiPreset] = useState(false); const [comboDragIndex, setComboDragIndex] = useState(null); @@ -1006,17 +1052,18 @@ function CombosPageContent() { }; const handleHideUsageGuideForever = () => { - setShowUsageGuide(false); try { globalThis.localStorage?.setItem(COMBO_USAGE_GUIDE_STORAGE_KEY, "1"); } catch {} + emitUsageGuideChange(); }; const handleShowUsageGuide = () => { - setShowUsageGuide(true); try { globalThis.localStorage?.removeItem(COMBO_USAGE_GUIDE_STORAGE_KEY); } catch {} + setUsageGuideHiddenForNow(false); + emitUsageGuideChange(); }; const handleFilterChange = (nextFilter) => { @@ -1149,7 +1196,7 @@ function CombosPageContent() { {showUsageGuide && ( setShowUsageGuide(false)} + onHide={() => setUsageGuideHiddenForNow(true)} onHideForever={handleHideUsageGuideForever} onCreateCombo={() => setShowCreateModal(true)} />