Skip to content

Eliminate setState-in-effect anti-patterns across 16 sites #200

Description

@JohnRDOrazio

Context

eslint-config-next 16 introduced the react-hooks/set-state-in-effect rule, which flags setState calls made synchronously inside a useEffect body. This pattern produces cascading renders, makes data flow harder to reason about, and is almost always a sign that the state can be derived, lifted, or assigned during the triggering event instead.

The rule is currently set to "warn" in eslint.config.mjs:9-11 so it doesn't gate CI:

// TODO: address these new react-hooks rules from eslint-config-next 16
// See: https://react.dev/learn/you-might-not-need-an-effect
"react-hooks/set-state-in-effect": "warn",
"react-hooks/immutability": "warn",

This issue tracks the cleanup of all current set-state-in-effect warnings so the rule can graduate to "error".

Scope

16 unique sites across 12 files. Grouping by remedy makes the work tractable — most of these are textbook patterns from React's you-might-not-need-an-effect docs.

Category A — Reset-on-prop-change (use key prop or derived state)

When a parent prop changes and a child resets its local state to match, the React-recommended fix is for the parent to give the child a different key prop so it remounts naturally — no effect required.

  • components/editor/AddEntityDialog.tsx:96 — reset dialog state when open flips
  • components/editor/AddEntityDialog.tsx:120setIri(generateIri(label)) derived from label; replace with useMemo or compute during render
  • components/editor/PropertyDetailPanel.tsx:260 — reset edit state when propertyIri changes
  • components/editor/PropertyDetailPanel.tsx:289 — restored-draft init when both restoredDraft and propertyIri are present
  • components/editor/DeleteImpactAnalysis.tsx:32 — reset state when entityIri/projectId change
  • app/page.tsx:75setNextPageError(null) when filter/debouncedSearch change; derive in render or move into the search-change handler
  • app/projects/[id]/editor/page.tsx:242setOntologyNamespace(defaultNs) derived from the prefix map; move to useMemo

Category B — Browser-API subscriptions (use useSyncExternalStore)

External stores (matchMedia, websockets) belong in useSyncExternalStore, not useEffect + setState.

  • components/editor/TurtleEditor.tsx:107setSystemTheme from window.matchMedia("(prefers-color-scheme: dark)") — textbook useSyncExternalStore candidate
  • lib/hooks/useCollaborationStatus.ts:129 — websocket connect/disconnect lifecycle; consider wrapping in a small store

Category C — Server-data-derived (overlaps with #89)

State mirrored from a query response. Best fix is to derive the value via React Query's select option, or compute it in render rather than mirroring into local state.

  • lib/context/BranchContext.tsx:116setCurrentBranch after response arrives
  • lib/context/BranchContext.tsx:149setInitialBranchHandled(true) after init; could be a ref or derived flag
  • app/projects/[id]/suggestions/page.tsx:83 — manual setIsLoadingSessions/setSessions/setSessionsError from a fetch; straightforward React Query migration candidate (cross-link with Migrate manual data fetching to React Query hooks #89)

Category D — One-shot init / event-driven UI (move to handler or useState initializer)

Work that runs once on mount or in response to a discrete event should live in a useState lazy initializer or in the handler that triggered it, not in an effect.

  • app/auth/error/page.tsx:39setRetryCount((c) => c + 1) inside countdown tick; move into the interval handler that also updates setCountdown
  • app/page.tsx:31 — one-shot auth-default-applied flag with ref guard; use a useState lazy initializer that reads isAuthenticated once
  • app/settings/page.tsx:342setHighlightedSetting(hash) for hash-scroll spy; move into the hash-listener handler
  • components/editor/shared/EntityTreeToolbar.tsx:52setShowTip(true) from localStorage read on mount; use useState(() => !localStorage.getItem(TIP_DISMISSED_KEY)) lazy initializer

Recommended order

  1. Category A first (7 sites). Largest cluster, single canonical fix (key prop or derivation), and exercises the test suite hardest because the affected components have substantial coverage.
  2. Category D (4 sites). Small, mostly mechanical fixes; gets the warning count down quickly.
  3. Category B (2 sites). useSyncExternalStore migration is more involved but well-isolated.
  4. Category C (3 sites). Best done alongside the Migrate manual data fetching to React Query hooks #89 React Query migration so the changes share PRs and review context — the suggestions-page site in particular is a natural addition to issue Migrate manual data fetching to React Query hooks #89's Phase 2.

Done criteria

  • All 16 sites refactored
  • react-hooks/set-state-in-effect upgraded from "warn" to "error" in eslint.config.mjs
  • react-hooks/immutability evaluated and either fixed or kept at "warn" with a documented reason
  • No new set-state-in-effect warnings introduced (CI now enforces)

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions