From 5d53f35dea7eead28a0cf0146d634fd15e168df2 Mon Sep 17 00:00:00 2001 From: Benjamin Sutter Date: Sun, 24 May 2026 01:30:44 +0200 Subject: [PATCH] refactor(state): clean layoutStore, add selectors, centralise STALE constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - layoutStore: remove 4 dead field groups (pinnedPanels, selectedResultId, compareTrayVisible, notificationsOpen) — none were read outside the store - CompareTray: drop dead useLayoutStore side-effect (state was write-only) - 8 components: replace bare useStore() with explicit selectors / useShallow to prevent unnecessary re-renders on unrelated state mutations - lib/constants: add STALE_MARKET_SIGNALS + STALE_REVIEW_QUEUE (30s each) - useMarketSignals, useReviewQueue: use global constants instead of hook-local magic numbers - Add STATE_MANAGEMENT.md: decision tree + rules for RQ/Zustand/local/derived Co-Authored-By: Claude Sonnet 4.6 --- STATE_MANAGEMENT.md | 223 ++++++++++++++++++ src/App.tsx | 2 +- .../assistant/GlobalAIAssistantButton.tsx | 3 +- .../assistant/GlobalAIAssistantDrawer.tsx | 3 +- src/components/layout/AppShell.tsx | 6 +- src/components/layout/CompareTray.tsx | 7 - src/components/layout/RightContextPanel.tsx | 4 +- src/components/supply/ReminderFeed.tsx | 9 +- src/components/supply/ReminderFilterBar.tsx | 9 +- src/hooks/useMarketSignals.ts | 7 +- src/hooks/useReviewQueue.ts | 6 +- src/lib/constants.ts | 2 + src/provider/AuthProvider.tsx | 5 +- src/stores/layoutStore.ts | 18 -- 14 files changed, 263 insertions(+), 41 deletions(-) create mode 100644 STATE_MANAGEMENT.md diff --git a/STATE_MANAGEMENT.md b/STATE_MANAGEMENT.md new file mode 100644 index 0000000..06ea9ce --- /dev/null +++ b/STATE_MANAGEMENT.md @@ -0,0 +1,223 @@ +# State Management — Property Match + +## Decision Tree + +``` +Is it server data (fetched from an API or provider)? + → React Query (useQuery / useMutation) + +Is it global UI state shared across unrelated components? + → Zustand store + +Is it local to a single component or parent-child chain? + → useState / useReducer (local state) + +Can it be computed from existing state/data? + → Derived state (compute inline — no separate store field) + +Is it cross-cutting auth / session context accessed in non-React code? + → Zustand store read via getState() (not useStore hook) +``` + +--- + +## React Query — Server State + +**Rule:** React Query owns all data that comes from a provider (mock or real). +Never copy React Query data into a Zustand store. + +### When to use + +- Fetching lists or detail records (`useQuery`) +- Creating, updating, deleting records (`useMutation`) +- Anything that needs cache invalidation or background refetch + +### Patterns + +```ts +// ✅ Correct — server data in React Query +const { data: properties } = useProperties() + +// ✅ Correct — mutation with cache invalidation +const createProp = useCreateProperty() +createProp.mutate(input, { + onSuccess: () => queryClient.invalidateQueries({ queryKey: ['properties'] }) +}) + +// ❌ Wrong — copying server data into a Zustand store +const [properties, setProperties] = useState([]) +useEffect(() => { fetchProperties().then(setProperties) }, []) +``` + +### staleTime constants + +All stale times live in `src/lib/constants.ts` — never define them locally in hooks. + +| Constant | Value | Used for | +|----------|-------|---------| +| `STALE_PROPERTIES` | 5 min | Property lists and details | +| `STALE_MATCHES` | 2 min | Match results (change with need edits) | +| `STALE_SIGNALS` | 5 min | Future availability signals | +| `STALE_MARKET_SIGNALS` | 30 s | Market intelligence (ops team, real-time) | +| `STALE_REVIEW_QUEUE` | 30 s | Review queue tasks (ops team, real-time) | + +### Query key conventions + +```ts +['entity'] // list: ['properties'], ['matches'], ['reminders'] +['entity', id] // single: ['property', id], ['match', id] +['entity', 'scope', id] // scoped: ['matches', 'need', needId] +['entity', filters] // filtered: ['properties', { assetType: 'OFFICE' }] +``` + +--- + +## Zustand — UI / Interaction State + +**Rule:** Zustand owns UI state only. It must never hold data that belongs in React Query. + +### When to use + +- Multi-step wizard state (`offerWizardStore`) +- Dialog open/close + pending item (`shortlistStore`, `pipelineStore`) +- Sidebar / layout flags (`layoutStore`) +- Auth session (`sessionStore` — special case, also read by services via `getState()`) +- Toast queue (`toastStore`) +- Active selection in a panel (`matchCenterStore`, `reminderStore`) + +### When NOT to use + +- Data fetched from a provider → use React Query +- State used only inside one component → use `useState` +- Values derived from existing state → compute inline + +### Selector rules + +Always use a selector. Subscribing to the full store causes re-renders on every state mutation. + +```ts +// ✅ Individual field selector — re-renders only when that field changes +const isOpen = useLayoutStore(s => s.isRightPanelOpen) +const close = useLayoutStore(s => s.closeRightPanel) + +// ✅ useShallow for multiple fields from the same store +import { useShallow } from 'zustand/react/shallow' +const { filterType, filterStatus } = useReminderStore( + useShallow(s => ({ filterType: s.filterType, filterStatus: s.filterStatus })) +) + +// ❌ No selector — subscribes to all store fields, re-renders on any change +const { isOpen, close } = useLayoutStore() + +// ✅ Non-React code (services, mutations) reads store via getState — no subscription +const user = useSessionStore.getState().currentUser +``` + +### Store inventory + +| Store | Responsibility | Key state | +|-------|---------------|-----------| +| `layoutStore` | App shell layout | `activeWorkspace`, `sidebarCollapsed`, `isRightPanelOpen` | +| `sessionStore` | Auth / current user | `currentUser`, `isAuthenticated`, `sessionStatus` | +| `assistantStore` | AI drawer conversation | `isOpen`, `messages`, `context`, `isLoading` | +| `compareStore` | Compare tray items | `compareItems[]` | +| `pipelineStore` | Pipeline items + add-dialog | `items[]`, `dialogOpen`, `pendingItem` | +| `shortlistStore` | Shortlist selection + add-dialog | `selectedShortlistId`, `dialogOpen`, `pendingItem` | +| `offerWizardStore` | Multi-step offer wizard | `isOpen`, `currentStep`, `selectedPropertyIds`, `editableFields` | +| `matchCenterStore` | Supply-side match center selection | `selectedPropertyId`, `selectedNeedId` | +| `reminderStore` | Reminder list filters + drawer | `filterType`, `filterStatus`, `selectedId`, `drawerOpen` | +| `toastStore` | Toast notification queue | `toasts[]` | + +### Adding a new store field + +Ask these questions first: +1. Is this server data? → React Query instead. +2. Is this only used in one component? → `useState` instead. +3. Is this derived from existing state? → compute it, don't store it. + +--- + +## Local State — Component-Scoped + +**Rule:** Default to `useState`. Only escalate to Zustand when state genuinely needs to be shared across unrelated components. + +### When to use + +- Form input values +- Toggle / accordion open state +- Hover / focus effects +- Step progress inside a self-contained wizard step +- Any state that resets when the component unmounts + +```ts +// ✅ Local — form input, no other component needs this +const [name, setName] = useState('') + +// ✅ Local — dialog only opened from one place +const [open, setOpen] = useState(false) + +// ❌ Should be local — extracted to store unnecessarily +// (e.g. a "confirmDialogOpen" only ever toggled from one parent) +``` + +--- + +## Derived State — Compute, Don't Store + +**Rule:** Never store a value that can be computed from existing state or query data. Compute it at render time. + +```ts +// ✅ Derived — compute from store +const isFull = compareItems.length >= MAX_COMPARE_ITEMS // NOT stored + +// ✅ Derived — compute from React Query data +const overdueReminders = reminders.filter(r => isPastDue(r.dueDate)) // NOT stored + +// ❌ Stored derived state — causes sync bugs +const [overdueCount, setOverdueCount] = useState(0) +useEffect(() => setOverdueCount(reminders.filter(...).length), [reminders]) +``` + +Exception: expensive computations (e.g. score calculation over thousands of items) may use `useMemo`. + +--- + +## Context — When Neither React Query nor Zustand Fits + +Use React Context for: +- Dependency injection (swap provider implementations) +- Tree-scoped state (e.g. a form context for nested inputs) +- Auth abstraction (`AuthProvider` wraps `sessionStore` so components don't import the store directly) + +Do NOT use Context as a replacement for React Query or Zustand — it causes cascading re-renders without cache or subscription granularity. + +--- + +## Service / Non-React Code + +Services must not import React hooks. They access Zustand state via `getState()`: + +```ts +// ✅ In a service — no hook, no subscription +const user = useSessionStore.getState().currentUser + +// ✅ In a React Query mutation onError +onError: () => useToastStore.getState().showToast('Fehler aufgetreten', 'error') + +// ❌ Services must never call useStore hooks +import { useSessionStore } from '../stores/sessionStore' +const { currentUser } = useSessionStore() // only valid inside a React component +``` + +--- + +## Anti-Patterns to Avoid + +| Anti-pattern | Why bad | Fix | +|---|---|---| +| `useStore()` without selector | Re-renders on every store mutation | Use `s => s.field` selector or `useShallow` | +| Server data in Zustand | Duplicates cache, causes stale/sync bugs | React Query | +| Derived state stored in state | Sync bugs, extra renders | Compute inline | +| Local dialog state in global store | Bloats store, breaks encapsulation | `useState` | +| Cross-store imports | Tight coupling, circular risk | Keep stores independent | +| Hook-local `STALE_*` constants | Inconsistent cache behaviour | Use `src/lib/constants.ts` | diff --git a/src/App.tsx b/src/App.tsx index 032815e..d71627e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -12,7 +12,7 @@ const WORKSPACE_HOME: Record = { } function RoleRedirect() { - const { currentUser } = useSessionStore() + const currentUser = useSessionStore(s => s.currentUser) const first = currentUser?.allowedWorkspaces[0] ?? WorkspaceType.SUPPLY return } diff --git a/src/components/assistant/GlobalAIAssistantButton.tsx b/src/components/assistant/GlobalAIAssistantButton.tsx index 94db088..278c382 100644 --- a/src/components/assistant/GlobalAIAssistantButton.tsx +++ b/src/components/assistant/GlobalAIAssistantButton.tsx @@ -3,7 +3,8 @@ import { Sparkles } from 'lucide-react' import { useAssistantStore } from '../../stores/assistantStore' export function GlobalAIAssistantButton() { - const { isOpen, open } = useAssistantStore() + const isOpen = useAssistantStore(s => s.isOpen) + const open = useAssistantStore(s => s.open) return ( diff --git a/src/components/assistant/GlobalAIAssistantDrawer.tsx b/src/components/assistant/GlobalAIAssistantDrawer.tsx index ea5ae73..38d5509 100644 --- a/src/components/assistant/GlobalAIAssistantDrawer.tsx +++ b/src/components/assistant/GlobalAIAssistantDrawer.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { Box, Divider, Drawer, IconButton, TextField, Tooltip, Typography } from '@mui/material' import { RotateCcw, Send, Sparkles, X } from 'lucide-react' import { useLocation } from 'react-router' +import { useShallow } from 'zustand/react/shallow' import { useAssistantStore } from '../../stores/assistantStore' import { useSessionStore } from '../../stores/sessionStore' import { aiAssistantService } from '../../services/aiAssistantService' @@ -21,7 +22,7 @@ function resolveWorkspace(pathname: string): WorkspaceType | null { export function GlobalAIAssistantDrawer() { const { isOpen, close, context, setContext, messages, isLoading, error, addMessage, setLoading, setError, clearConversation } = - useAssistantStore() + useAssistantStore(useShallow(s => s)) const { currentUser } = useSessionStore() const location = useLocation() diff --git a/src/components/layout/AppShell.tsx b/src/components/layout/AppShell.tsx index 4bd0a74..f32cdf6 100644 --- a/src/components/layout/AppShell.tsx +++ b/src/components/layout/AppShell.tsx @@ -17,8 +17,10 @@ import { WorkspaceType } from '../../domain/enums' // --------------------------------------------------------------------------- export function AppShell() { - const { activeWorkspace, sidebarCollapsed, setActiveWorkspace, toggleSidebar } = - useLayoutStore() + const activeWorkspace = useLayoutStore(s => s.activeWorkspace) + const sidebarCollapsed = useLayoutStore(s => s.sidebarCollapsed) + const setActiveWorkspace = useLayoutStore(s => s.setActiveWorkspace) + const toggleSidebar = useLayoutStore(s => s.toggleSidebar) const { currentUser } = useSessionStore() const navigate = useNavigate() const location = useLocation() diff --git a/src/components/layout/CompareTray.tsx b/src/components/layout/CompareTray.tsx index 0ea3356..ac1f610 100644 --- a/src/components/layout/CompareTray.tsx +++ b/src/components/layout/CompareTray.tsx @@ -1,9 +1,7 @@ -import { useEffect } from 'react' import { useNavigate, useLocation } from 'react-router' import { Box, Button, IconButton, Typography } from '@mui/material' import { X } from 'lucide-react' import { useCompareStore } from '../../stores/compareStore' -import { useLayoutStore } from '../../stores/layoutStore' const TYPE_DOT: Record = { VERIFIED_PORTFOLIO: '#1e3a5f', @@ -14,15 +12,10 @@ const TYPE_DOT: Record = { export function CompareTray() { const { compareItems, removeFromCompare, clearCompare } = useCompareStore() - const { setCompareTrayVisible } = useLayoutStore() const navigate = useNavigate() const location = useLocation() const isDemand = location.pathname.startsWith('/demand') - useEffect(() => { - setCompareTrayVisible(compareItems.length > 0 && isDemand) - }, [compareItems.length, setCompareTrayVisible, isDemand]) - if (!isDemand) return null const getTitle = (item: (typeof compareItems)[number]) => { diff --git a/src/components/layout/RightContextPanel.tsx b/src/components/layout/RightContextPanel.tsx index cb6d123..6222df3 100644 --- a/src/components/layout/RightContextPanel.tsx +++ b/src/components/layout/RightContextPanel.tsx @@ -18,7 +18,9 @@ const PANEL_PLACEHOLDERS: Record = { } export function RightContextPanel() { - const { isRightPanelOpen, rightPanelContentType, closeRightPanel } = useLayoutStore() + const isRightPanelOpen = useLayoutStore(s => s.isRightPanelOpen) + const rightPanelContentType = useLayoutStore(s => s.rightPanelContentType) + const closeRightPanel = useLayoutStore(s => s.closeRightPanel) const title = rightPanelContentType ? PANEL_TITLES[rightPanelContentType] : '' const placeholder = rightPanelContentType ? PANEL_PLACEHOLDERS[rightPanelContentType] : '' diff --git a/src/components/supply/ReminderFeed.tsx b/src/components/supply/ReminderFeed.tsx index 43d2a45..111b391 100644 --- a/src/components/supply/ReminderFeed.tsx +++ b/src/components/supply/ReminderFeed.tsx @@ -1,5 +1,6 @@ import { Box, Typography } from '@mui/material' import { useReminders } from '../../hooks/useReminders' +import { useShallow } from 'zustand/react/shallow' import { useReminderStore } from '../../stores/reminderStore' import { ReminderListRow } from './ReminderListRow' import { ReminderCard } from './ReminderCard' @@ -34,7 +35,13 @@ export function ReminderFeed() { const { filterType, filterStatus, filterPriority, searchQuery, viewMode, setFilterType, setFilterStatus, setFilterPriority, setSearchQuery, - } = useReminderStore() + } = useReminderStore(useShallow(s => ({ + filterType: s.filterType, filterStatus: s.filterStatus, + filterPriority: s.filterPriority, searchQuery: s.searchQuery, + viewMode: s.viewMode, setFilterType: s.setFilterType, + setFilterStatus: s.setFilterStatus, setFilterPriority: s.setFilterPriority, + setSearchQuery: s.setSearchQuery, + }))) if (isLoading) return diff --git a/src/components/supply/ReminderFilterBar.tsx b/src/components/supply/ReminderFilterBar.tsx index 4415280..b9fe3ec 100644 --- a/src/components/supply/ReminderFilterBar.tsx +++ b/src/components/supply/ReminderFilterBar.tsx @@ -1,5 +1,6 @@ import { Box, ToggleButtonGroup, ToggleButton, TextField, InputAdornment, Typography } from '@mui/material' import { Search } from 'lucide-react' +import { useShallow } from 'zustand/react/shallow' import { useReminderStore } from '../../stores/reminderStore' import { ReminderType, ReminderStatus, ReminderPriority } from '../../domain/reminder' import type { ReminderType as ReminderTypeType, ReminderStatus as ReminderStatusType, ReminderPriority as ReminderPriorityType } from '../../domain/reminder' @@ -36,7 +37,13 @@ export function ReminderFilterBar() { filterPriority, setFilterPriority, searchQuery, setSearchQuery, viewMode, setViewMode, - } = useReminderStore() + } = useReminderStore(useShallow(s => ({ + filterType: s.filterType, setFilterType: s.setFilterType, + filterStatus: s.filterStatus, setFilterStatus: s.setFilterStatus, + filterPriority: s.filterPriority, setFilterPriority: s.setFilterPriority, + searchQuery: s.searchQuery, setSearchQuery: s.setSearchQuery, + viewMode: s.viewMode, setViewMode: s.setViewMode, + }))) return ( diff --git a/src/hooks/useMarketSignals.ts b/src/hooks/useMarketSignals.ts index 07d1132..ef0a4a5 100644 --- a/src/hooks/useMarketSignals.ts +++ b/src/hooks/useMarketSignals.ts @@ -3,14 +3,13 @@ import { marketIntelligenceService } from '../services/marketIntelligenceService import { reviewService } from '../services/reviewService' import type { MarketSignalFilters, SignalProcessingStatus } from '../domain/marketSignal' import { useToastStore } from '../stores/toastStore' - -const STALE_SIGNALS = 30_000 +import { STALE_MARKET_SIGNALS } from '../lib/constants' export function useMarketSignals(filters?: MarketSignalFilters) { return useQuery({ queryKey: ['market-signals', filters ?? {}], queryFn: () => marketIntelligenceService.getSignals(filters), - staleTime: STALE_SIGNALS, + staleTime: STALE_MARKET_SIGNALS, select: (res) => res.data ?? [], }) } @@ -20,7 +19,7 @@ export function useMarketSignalDetail(id: string | null) { queryKey: ['market-signal', id], queryFn: () => marketIntelligenceService.getSignalDetail(id!), enabled: id !== null, - staleTime: STALE_SIGNALS, + staleTime: STALE_MARKET_SIGNALS, select: (res) => res.data ?? null, }) } diff --git a/src/hooks/useReviewQueue.ts b/src/hooks/useReviewQueue.ts index c89215c..78638b7 100644 --- a/src/hooks/useReviewQueue.ts +++ b/src/hooks/useReviewQueue.ts @@ -4,15 +4,15 @@ import { useSessionStore } from '../stores/sessionStore' import type { ReviewFilters } from '../provider/IReviewProvider' import type { ReviewTaskStatus } from '../domain/review' import { useToastStore } from '../stores/toastStore' +import { STALE_REVIEW_QUEUE } from '../lib/constants' -const STALE_REVIEW = 30_000 const QK = 'reviewQueue' export function useReviewQueue(filters?: ReviewFilters) { return useQuery({ queryKey: [QK, filters ?? {}], queryFn: () => reviewService.getTasks(filters), - staleTime: STALE_REVIEW, + staleTime: STALE_REVIEW_QUEUE, select: (res) => res.data ?? [], }) } @@ -21,7 +21,7 @@ export function useReviewTask(id: string | null) { return useQuery({ queryKey: [QK, 'task', id], queryFn: () => reviewService.getTask(id!), - staleTime: STALE_REVIEW, + staleTime: STALE_REVIEW_QUEUE, enabled: !!id, select: (res) => res.data ?? null, }) diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 364f30b..55f19e7 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -30,6 +30,8 @@ export const PROB_MEDIUM = 0.5 export const STALE_PROPERTIES = 5 * 60 * 1000 export const STALE_MATCHES = 2 * 60 * 1000 export const STALE_SIGNALS = 5 * 60 * 1000 +export const STALE_MARKET_SIGNALS = 30 * 1000 // aggressive — market leads refresh often +export const STALE_REVIEW_QUEUE = 30 * 1000 // aggressive — ops team works in real-time // Route paths — single source of truth export const ROUTES = { diff --git a/src/provider/AuthProvider.tsx b/src/provider/AuthProvider.tsx index c69d396..7920b25 100644 --- a/src/provider/AuthProvider.tsx +++ b/src/provider/AuthProvider.tsx @@ -16,7 +16,10 @@ interface AuthContextValue { const AuthContext = createContext(null) export function AuthProvider({ children }: { children: ReactNode }) { - const { currentUser, isAuthenticated, login, logout } = useSessionStore() + const currentUser = useSessionStore(s => s.currentUser) + const isAuthenticated = useSessionStore(s => s.isAuthenticated) + const login = useSessionStore(s => s.login) + const logout = useSessionStore(s => s.logout) const value: AuthContextValue = { user: currentUser, diff --git a/src/stores/layoutStore.ts b/src/stores/layoutStore.ts index e3b8d55..9e55cf9 100644 --- a/src/stores/layoutStore.ts +++ b/src/stores/layoutStore.ts @@ -12,39 +12,24 @@ export type RightPanelContentType = typeof RightPanelContentType[keyof typeof Ri interface LayoutState { activeWorkspace: WorkspaceType sidebarCollapsed: boolean - pinnedPanels: string[] isRightPanelOpen: boolean rightPanelContentType: RightPanelContentType | null - compareTrayVisible: boolean - selectedResultId: string | null - notificationsOpen: boolean // Actions setActiveWorkspace: (workspace: WorkspaceType) => void toggleSidebar: () => void - pinPanel: (panelId: string) => void - unpinPanel: (panelId: string) => void openRightPanel: (type: RightPanelContentType) => void closeRightPanel: () => void toggleRightPanel: (type: RightPanelContentType) => void - setCompareTrayVisible: (visible: boolean) => void - setSelectedResultId: (id: string | null) => void - toggleNotifications: () => void } export const useLayoutStore = create((set, get) => ({ activeWorkspace: WorkspaceType.SUPPLY, sidebarCollapsed: false, - pinnedPanels: [], isRightPanelOpen: false, rightPanelContentType: null, - compareTrayVisible: false, - selectedResultId: null, - notificationsOpen: false, setActiveWorkspace: (workspace) => set({ activeWorkspace: workspace }), toggleSidebar: () => set((s) => ({ sidebarCollapsed: !s.sidebarCollapsed })), - pinPanel: (panelId) => set((s) => ({ pinnedPanels: [...s.pinnedPanels, panelId] })), - unpinPanel: (panelId) => set((s) => ({ pinnedPanels: s.pinnedPanels.filter(id => id !== panelId) })), openRightPanel: (type) => set({ isRightPanelOpen: true, rightPanelContentType: type }), closeRightPanel: () => set({ isRightPanelOpen: false, rightPanelContentType: null }), toggleRightPanel: (type) => { @@ -55,7 +40,4 @@ export const useLayoutStore = create((set, get) => ({ set({ isRightPanelOpen: true, rightPanelContentType: type }) } }, - setCompareTrayVisible: (visible) => set({ compareTrayVisible: visible }), - setSelectedResultId: (id) => set({ selectedResultId: id }), - toggleNotifications: () => set((s) => ({ notificationsOpen: !s.notificationsOpen })), }))