Skip to content

feat: eliminate direct useEffect — adopt declarative React patterns #922

@ryaneggz

Description

@ryaneggz

Metadata

IMPORTANT: The very first step should ALWAYS be validating this metadata section to maintain a CLEAN development workflow.

pull_request_title: "feat: eliminate direct useEffect — adopt declarative React patterns"
branch: "feat/useeffect-refactor"

User Stories

  • As a developer, I want useEffect calls replaced with declarative patterns (TanStack Query, useMountEffect, derived state, key props) so that the codebase has fewer hidden coupling issues, infinite loops, and race conditions.
  • As a developer, I want a useMountEffect hook for one-time setup so that mount-only effects are semantically clear and the eslint suppression is centralized.
  • As a developer, I want data fetching managed by TanStack Query so that loading/error states, caching, and cache invalidation are handled declaratively.

Summary

Based on the "Why we banned useEffect" approach, this issue tracks refactoring ~118 direct useEffect calls across 83 frontend files into 5 declarative patterns:

  1. Derive state, don't sync ituseMemo / inline calculation
  2. Use data-fetching libraries — TanStack Query (@tanstack/react-query)
  3. Event handlers, not effects — handle actions directly in handlers
  4. useMountEffect for external sync — semantic wrapper for useEffect(fn, [])
  5. Reset with key, not choreography — React key prop for clean remounts

Visual Reference

Tweet: https://x.com/alvinsng/status/2033969062834045089


Key Integration Points

File Function(s) Role
frontend/src/hooks/useAuth.tsx useEffectGetUser() Auth state — replace with useQuery
frontend/src/hooks/useModel.tsx useModelsEffect() Model list — replace with useQuery
frontend/src/hooks/useAgent.ts useEffectGetAgents(), state-sync effect Agent data + model sync — replace with useQuery, remove sync
frontend/src/hooks/useThread.ts useListThreadsEffect(), useLoadThreadEffect() Thread data — replace with useQuery/useInfiniteQuery
frontend/src/hooks/useChat.ts useEffectUpdateAssistantId() Metadata sync — remove (redundant with getMetadata())
frontend/src/context/ChatContext.tsx 7 effects (file sync, autosave, chains) Effect chains — break with useMemo, useMountEffect, custom hooks

UI Integration Points

Component / Route Change Type Description
src/components/settings/*Settings.tsx (6 files) Modify Replace fetch effects with useQuery
src/pages/threads/ThreadPage.tsx Modify Key-reset pattern with key={threadId}
src/components/panels/FileEditorPanel.tsx Modify Derive state instead of syncing via effects
src/pages/agents/edit.tsx, thread.tsx Modify Key-reset + useMountEffect for cleanup

Storage

  • Persistence layer: No changes — existing API services remain the data layer
  • Caching: TanStack Query in-memory cache replaces manual useState for server data
  • Model pattern: Existing service layer (src/lib/services/) unchanged

Architectural Decisions

  • Source of truth: TanStack Query cache for server state, React state for UI state
  • State management: useQuery for reads, useMutation + invalidateQueries for writes
  • No Redux/Zustand: Continue using React Context for cross-cutting UI state
  • Incremental rollout: 7 phases across 6-9 PRs, each self-contained and revertible

Documentation


Development Setup

Dependencies

Dependency Version Notes
@tanstack/react-query v5 Core data-fetching library
@tanstack/react-query-devtools v5 Dev-only debugging panel

Commands

cd frontend && npm install
npm run test
npm run dev:claude

Design Principles

  • Simplicity is beauty, complexity is pain.
  • ALWAYS look at the current codebase first — achieve the goal in the least amount of changes.
  • TDD-first: write tests before implementation.
  • Effects are for synchronizing with external systems, not for deriving state or fetching data.
  • Every useEffect must justify its existence — if it can be a useMemo, event handler, or key prop, it should be.

Validation Tools

  • Load agent-browser skill with screenshots to validate E2E
  • npm run test passes after each phase
  • npm run lint passes
  • Manual smoke test: chat, switch threads, edit agent, settings page

Acceptance Criteria

  • TanStack Query v5 installed and QueryProvider wrapping the app
  • useMountEffect hook created and used for all mount-only effects
  • Query key factory created at src/lib/queryKeys.ts
  • All useEffectXxx() wrapper patterns eliminated from hooks
  • Settings components use useQuery instead of manual fetch effects
  • ThreadPage uses key={threadId} instead of state-reset effect
  • FileEditorPanel derives state instead of syncing via effects
  • ChatContext effect chains broken into useMemo + useMountEffect
  • All previous & new tests pass
  • No regressions in chat, thread loading, agent editing, or settings

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions