Files
5chan/docs/agent-runs/codebase-audit-2026-04-23/02-state-management.md
T
Tommaso CasaburiandGitHub 5dc5408a15 fix(codebase audit): preserve cleanup without regressions
Fix codebase audit regressions while preserving UI/UX behavior and adding review-driven hardening.
2026-04-24 15:48:07 +07:00

94 lines
20 KiB
Markdown

# State Management Audit
## Summary
The Zustand layer is mostly a loose collection of 32 single-purpose stores, many of which are healthy (tiny modal stores, preference toggles). The biggest systemic problems are: (1) near-universal whole-store destructuring in consumers — there is no `useShallow` anywhere in the codebase, so every call to `const { a, b } = useXStore()` re-renders on any state change, (2) `use-catalog-filters-store` has ballooned into a 400 LOC kitchen-sink with a live filter function cached in state, a `setTimeout` that mutates state from a render-time selector path, and an imperative top-level self-kick on module import, and (3) ~7 stores roll their own ad-hoc localStorage handlers instead of using the `persist` middleware that the project already depends on.
## Store Inventory
| Store | Responsibility | LOC | Main concern |
|---|---|---|---|
| `use-catalog-filters-store.ts` | Catalog filter text list, per-community counts, search, match highlights | 400 | Kitchen-sink; stores a `(comment) => boolean` function; `setTimeout` side-effect inside selector; top-level `.getState().updateFilter()` at module load |
| `use-boards-bar-visibility-store.ts` | Boards-bar directory/subscription visibility | 128 | Hand-rolled localStorage reads/writes (4 keys + legacy fallbacks); should use `persist` |
| `use-reply-modal-store.ts` | Reply modal open state + quote-insertion signals | 117 | Mixes transient reply target (cid/number/community) and quote-insert event stream; DOM read (`window.innerWidth`) inside store action; cross-store write to `useSelectedTextStore` on close |
| `use-mod-queue-store.ts` | Mod queue alert threshold, dismissed cids, queue history, view mode, selected board | 115 | Mixes unrelated concerns: ephemeral UI (`viewMode`, `selectedBoardFilter`) with persisted data (`queuedCommentHistory`, `dismissedCommentCids`, thresholds); action `getAlertThresholdSeconds` is a derived getter masquerading as a method |
| `use-post-number-store.ts` | Post number <-> cid maps keyed by community | 86 | Reasonable; but `getScopedNumberToCidMap` is a non-trivial selector computed outside memoization boundary |
| `use-publish-post-store.ts` | Draft state for a single top-level post | 85 | Singleton draft; ties publish-option construction to state shape; `publishCommentOptions` cached in state duplicates fields already in state |
| `use-disclaimer-modal-store.ts` | NSFW disclaimer modal + acceptance | 84 | Takes a `NavigateFunction` into store actions (coupling); hand-rolled localStorage |
| `use-publish-reply-store.ts` | Draft reply state keyed by parentCid | 80 | 6 parallel dictionaries keyed by cid (should be one map of objects); leaks memory — `resetPublishReplyStore` sets `undefined` but never deletes keys |
| `use-feed-cache-store.ts` | LRU of recent feed keys (size 2) | 61 | Tiny LRU over an array; `maxCacheSize` is state but never changed |
| `use-theme-store.ts` | Current theme per sfw/nsfw category | 56 | Side-effect `loadThemes()` called at import; `getTheme` is an action that also mutates state (`currentTheme`) |
| `use-community-offline-store.ts` | Per-community offline state + 30s initial-load window | 52 | `setTimeout(..., 30_000)` inside store action leaks if community unmounts; `initialLoad` flag is derivable from timestamp |
| `use-boards-filter-store.ts` | Boards page sfw/nsfw filter + catalog-link toggle | 51 | Hand-rolled localStorage; two unrelated concerns in one store |
| `use-media-hosting-store.ts` | Media host provider + upload mode | 46 | Good — uses `persist` with version+migrate |
| `use-communities-loading-start-timestamps-store.ts` | First-seen timestamp per community | 44 | Exports a hook wrapper with its own `useEffect` + `useMemo`; selector returns a new array every render (`.map()`), defeats memo |
| `use-app-update-store.ts` | App update availability/apply state | 41 | Clean; minor — 3 booleans where a discriminated `status` would be clearer |
| `use-thread-live-updates-store.ts` | Thread auto-update enable + request id counter | 40 | Clean |
| `use-challenges-store.ts` | Queue of active challenges | 38 | Module-level `nextChallengeId` mutable counter outside store |
| `use-external-quote-status-store.ts` | Temporary error banner for external quote resolution | 37 | Module-level `hideTimeout` outside store |
| `use-special-theme-store.ts` | Christmas theme toggle | 34 | Clean |
| `use-publish-post-store.ts`+`use-publish-reply-store.ts` | — | — | Two stores that are 90% the same logic |
| `use-post-number-store.ts` | — | — | See above |
| `use-popular-threads-options-store.ts` | Home popular-threads sfw/nsfw toggles | 24 | Hand-rolled localStorage; imported under a misleading `useHomeFiltersStore` alias |
| `use-directory-modal-store.ts` | Directory modal open/close | 26 | Duplicate of two other modal stores |
| `use-create-board-modal-store.ts` | Create-board modal open/close | 26 | Duplicate |
| `use-boards-bar-edit-modal-store.ts` | Boards-bar edit modal open/close | 25 | Duplicate |
| `use-blotter-visibility-store.ts` | Blotter show/hide | 25 | Clean — uses `persist` |
| `use-expanded-media-store.ts` | Image fit + video mute prefs | 26 | Clean — uses `persist` |
| `use-feed-view-settings-store.ts` | Infinite-scroll toggle | 22 | Clean — uses `persist` |
| `use-catalog-style-store.ts` | Image size + show OP comment | 24 | Hand-rolled localStorage |
| `use-all-feed-filter-store.ts` | All-feed sfw/nsfw filter | 32 | Hand-rolled localStorage |
| `use-sorting-store.ts` | Catalog sort type | 17 | Clean |
| `use-feed-reset-store.ts` | Stores a reset function reference | 14 | Stores a function — this is a ref/pub-sub, not state |
| `use-selected-text-store.ts` | Selected-text string | 16 | Clean, but could be `useReplyModalStore` field |
## Findings
### Critical
- **`src/stores/use-catalog-filters-store.ts:206-250`** — `updateFilter` stores a new `(comment) => boolean` closure inside the store state. The closure captures `state` via `set((state) => ({ filter: ... }))`, so every subsequent mutation to `filterItems`, `searchText`, or `currentCommunityAddress` requires re-running `updateFilter()` to refresh the closure. Missing any of those paths produces a stale filter. Additionally, inside the filter at line 232 it calls `setTimeout(...)` to schedule `incrementFilterCount`, which mutates the store **from render-time consumers** (the filter is called during a render pass by whoever uses it). That is a store mutation as a render side-effect — exactly the Zustand anti-pattern the React 19 rules call out. *Fix:* stop storing a closure in state. Expose `matchesFilter(comment, communityAddress)` as a pure selector (or a utility that takes `filterItems` + `searchText` as arguments). Track per-community filtered cids in a separate update path triggered by the owning component after it has filtered its own list, not inside a predicate called during render.
- **`src/stores/use-catalog-filters-store.ts:398`** — Top-level `useCatalogFiltersStore.getState().updateFilter();` runs at module import. This executes before React is mounted and before the `persist` rehydration completes, so the initial filter closure sees empty `filterItems` even though persisted filters exist. Consumers then get a stale filter until another action happens to call `updateFilter`. *Fix:* trigger `updateFilter` from `onRehydrateStorage`, or replace with a pure selector so there is nothing to prime.
- **`src/stores/use-catalog-filters-store.ts:361-370`** — `partialize` drops all non-text fields (`count`, `filteredCids`, `communityCounts`, `communityFilteredCids`) but the `deserialize` migration on the next load calls `normalizeFilterItem` which re-creates empty `Map`/`Set` instances. That is fine, but the `deserialize` function never accounts for the version, and the branch at line 372 does `JSON.parse` manually — but the default `persist` storage already parses. On repeat calls it is parsing a plain object as a string, swallowing the error in the `typeof persisted === 'string'` check. *Fix:* drop the custom `deserialize` entirely; persisted data is already plain strings, and `Map`/`Set` revival should go through `storage: createJSONStorage(() => localStorage, { reviver, replacer })`.
### High
- **`src/app.tsx:183`** — `GlobalLayout` destructures 8 fields off `useReplyModalStore()` with no selector. This is a top-level always-mounted component; it re-renders every time any field (`quoteInsertRequestId` counter, `openEmpty`, `scrollY`, etc.) changes. *Fix:* split into per-field `useReplyModalStore((s) => s.activeCid)` selectors (already the pattern used inside `reply-modal.tsx` itself) or use `useShallow` on the destructured object.
- **`src/components/catalog-filters/catalog-filters.tsx:50-54`, `src/hooks/use-publish-post.ts:12-19`, `src/hooks/use-publish-reply.ts:25-31`** — Selectors that return a new object literal `(state) => ({ a: state.a, b: state.b })` without `useShallow`. Zustand compares the selector result with `Object.is` by default, so the new object is never equal and the hook re-fires on every store change. *Fix:* wrap with `useShallow` from `zustand/react/shallow`, or split into one-field selectors.
- **`src/stores/use-catalog-filters-store.ts:25`, `205`, `208`** — `filter: ((comment: Comment) => boolean) | undefined` is a function stored in state. Every consumer that reads `state.filter` re-subscribes to identity of this function. Combined with `updateFilter` rebuilding it on every filter/search/community change, anything reading it re-renders unnecessarily. *Fix:* as above, make it a pure selector — `selectCatalogFilterPredicate(state)` returns a stable-ish function based on current state, memoized via `zustand/middleware` or Zustand's `createSelector` equivalent.
- **`src/stores/use-mod-queue-store.ts:9-23`** — One store carries (a) persisted moderation data (dismissed cids, 500-entry history, thresholds) and (b) ephemeral UI (`viewMode: 'compact'|'feed'`, `selectedBoardFilter`). Every `rememberCommentsInQueue` mutation re-renders every consumer of `viewMode` across `board-buttons.tsx` and `mod-queue.tsx`. *Fix:* split into `use-mod-queue-data-store` (persisted) and `use-mod-queue-view-store` (session UI); alternatively, partialize the persist layer so UI fields are not saved and subscribers can select precisely.
- **`src/stores/use-publish-reply-store.ts:8-13, 69-77`** — State shape is 6 parallel `{ [parentCid]: T }` dictionaries. Any reply draft update spreads all 6 objects. Worse, `resetPublishReplyStore` sets each `[parentCid]: undefined` instead of deleting, so the dictionaries grow unboundedly for the lifetime of the session. *Fix:* collapse to `drafts: Record<string, ReplyDraft>` with a single `setDraft(parentCid, patch)` and `deleteDraft(parentCid)` that does `const { [parentCid]: _, ...rest } = state.drafts`.
- **`src/stores/use-reply-modal-store.ts:82-95`** — `window.innerWidth` read inside the store action; the store action is therefore non-deterministic across test environments and SSR. *Fix:* move the mobile check to the caller (there is already a `use-is-mobile` hook), and accept a `scrollY` argument.
- **`src/stores/use-reply-modal-store.ts:53, 79, 99`** — `useReplyModalStore` writes into `useSelectedTextStore` via `.getState()`. Bidirectional coupling: the two stores are really one feature. *Fix:* merge `selectedText` into `useReplyModalStore` (it is only ever used by the reply modal), and delete `use-selected-text-store.ts`.
### Medium
- **`src/stores/use-community-offline-store.ts:38-48`** — `initializeCommunityOfflineState` schedules a naked `setTimeout(..., 30_000)` that mutates the store. If the user navigates away, the timer still fires and flips `initialLoad: false` on unmounted communities. And `initialLoad` itself is derivable — `Date.now()/1000 - loadingStartTimestamp < 30` (the hook already computes this at `use-is-community-offline.ts:36`). *Fix:* delete `initialLoad` and the timer; derive from the already-persisted timestamp in `use-communities-loading-start-timestamps-store`.
- **`src/stores/use-communities-loading-start-timestamps-store.ts:37-39`** — `communitiesLoadingStartTimestamps` selector returns `communityAddresses.map(addr => timestampsStore[addr])` — a new array every render even when unchanged. The outer `useMemo` prevents re-renders of *this* hook but its consumers still see a new array reference each time `timestampsStore` mutates for any address. *Fix:* select the specific timestamps via `useShallow`, or compute a stable signature outside.
- **`src/stores/use-boards-bar-visibility-store.ts` (whole file)**, **`src/stores/use-boards-filter-store.ts`**, **`src/stores/use-catalog-style-store.ts`**, **`src/stores/use-all-feed-filter-store.ts`**, **`src/stores/use-popular-threads-options-store.ts`**, **`src/stores/use-disclaimer-modal-store.ts`** — All hand-roll `localStorage.getItem/setItem` inside store definitions instead of using the `persist` middleware that's already imported elsewhere. This multiplies the serialization bugs (e.g. `use-boards-bar-visibility-store.ts:54` parses an array as a boolean; `use-popular-threads-options-store.ts:11` has the "default true unless false" logic duplicated). *Fix:* migrate to `persist({ name, version, migrate, partialize })`. The existing keys can be mapped via `storage: createJSONStorage(() => localStorage)` with a `migrate` fn to copy from the legacy keys.
- **`src/stores/use-disclaimer-modal-store.ts:10, 36, 63`** — Store actions accept a React Router `NavigateFunction`. That makes the store coupled to the call site's router context and forces the component to thread navigate through props. *Fix:* have the store expose target state (`acceptedTargetPath`) and let a tiny `useDisclaimerNavigation` hook subscribe and call `navigate`.
- **`src/stores/use-theme-store.ts:32-39`** — `getTheme` is named like a selector but also mutates state (`set({ currentTheme: theme })`). Plus there's a `updateCurrentTheme=true` parameter that silently toggles side-effect behavior. *Fix:* split into a pure `getTheme(category)` selector and an explicit `setCurrentTheme` action.
- **`src/stores/use-theme-store.ts:54`** — `useThemeStore.getState().loadThemes()` runs at import time. `loadThemes` is async (reads from `localforage`). Any component that renders before that promise resolves sees the default `{ nsfw: 'yotsuba', sfw: 'yotsuba-b' }`, flickers, then re-renders when the real theme loads. *Fix:* expose an `isLoaded` flag so `useTheme()` can gate rendering, or call `loadThemes` from the root provider where the hydration lifecycle is explicit.
- **`src/stores/use-publish-post-store.ts:28-71`** — Store holds both the raw draft fields (`title`, `content`, `link`) **and** a pre-built `publishCommentOptions` derived from those fields. They can drift. And `setPublishPostStore` re-derives the options on every keystroke even though `use-publish-post.ts:73-81` wraps them again with `useMemo` + `onChallenge`. *Fix:* keep only raw draft fields in state; compute `publishCommentOptions` in `use-publish-post.ts` via `useMemo`.
- **`src/stores/use-boards-bar-visibility-store.ts:77-82`** — Store loads `getAllBoardCodes()` on first-render of any consumer. If the board-codes module changes size between releases (which it does; 5chan ships a new board list regularly), existing users keep their old set and new boards are invisibly hidden. *Fix:* store only the *hidden* set, not the visible one; default "visible" becomes "not in hidden set", and new boards appear automatically.
- **`src/stores/use-reply-modal-store.ts:14-16, 65-96`** — Event-stream fields (`quoteInsertRequestId`, `quoteInsertNumber`, `quoteInsertSelectedText`) are abusing persisted state as a pub/sub channel. Consumers (`reply-modal.tsx:59-61`) diff the request id in a `useEffect` to react to the event. *Fix:* either use a proper event emitter/ref, or at minimum document the pattern; consider a `useReplyQuoteInsertEvents` subscription helper.
- **`src/components/boards-bar/boards-bar.tsx:100-103`** — 4 modal stores opened in one component. These three modal stores (`directory`, `create-board`, `boards-bar-edit`) and `disclaimer` are structurally identical. *Fix:* one `useModalsStore` with `openModal(name)` / `closeModal()` — drops ~80 LOC and three files.
### Low
- **`src/stores/use-challenges-store.ts:4`** — `let nextChallengeId = 0;` is module-level mutable state that survives HMR inconsistently. *Fix:* move into the store.
- **`src/stores/use-external-quote-status-store.ts:9`** — `let hideTimeout` at module scope, same concern. *Fix:* move into state or use a ref stored in the store.
- **`src/stores/use-popular-threads-options-store.ts`** is aliased as `useHomeFiltersStore` in `src/views/home/box-modal/box-modal.tsx:3`. Misleading. *Fix:* rename the store or the import.
- **`src/stores/use-feed-reset-store.ts`** — stores a function to cross-cut-call "reset the current feed". This is a ref/command bus, not state. *Fix:* Zustand is OK for this, but document it, or switch to a simple module-level `{ reset?: () => void }` with a setter to avoid the re-render on every subscribe.
- **`src/stores/use-catalog-filters-store.ts:311-325`** — `getFilteredCountForCurrentCommunity` is a pure function over state but it is an action on the store. Callers that depend on it won't re-render when state changes. *Fix:* expose as a selector `selectFilteredCountForCurrentCommunity(state)`.
- **`src/stores/use-mod-queue-store.ts:77-80`** — Same pattern: `getAlertThresholdSeconds` is a pure derivation (`value * 3600` or `value * 60`) stored as an action. Callers like `post-desktop.tsx:247` read it at render and compute against it, missing re-renders when the threshold changes. *Fix:* make it a selector `(s) => s.alertThresholdUnit === 'hours' ? s.alertThresholdValue * 3600 : s.alertThresholdValue * 60`.
- **`src/stores/use-app-update-store.ts:6-10`** — `isApplyingUpdate` + `isCheckingForUpdate` + `availableUpdate` is flag soup; only one of (`idle`, `checking`, `available`, `applying`) is true at a time. *Fix:* a discriminated `status` union.
- **`src/stores/use-feed-cache-store.ts:22-42`** — `accessFeed` manually sorts + slices an array to simulate an LRU. With `maxCacheSize: 2`, this is fine in practice but the sort-slice semantics are wrong for size > 2 (keeps oldest plus newest). *Fix:* replace with `Map` insertion-order semantics (delete + re-set).
## Top 5 Actions
1. **Fix `use-catalog-filters-store`** (Critical). Remove the stored closure `filter`, remove the `setTimeout` that mutates state from inside a predicate, and delete the module-level `.getState().updateFilter()` primer. Make filtering a pure selector over `filterItems`/`searchText`/`communityAddress`, and move per-community counting into the catalog view where filtering already happens. This one store change eliminates the biggest re-render and correctness hazards in the codebase.
2. **Introduce `useShallow` as the default pattern for multi-field reads.** Add it to `src/app.tsx:183` (Reply modal wrapping everything), `src/components/catalog-filters/catalog-filters.tsx:50`, `src/hooks/use-publish-post.ts:12`, `src/hooks/use-publish-reply.ts:25`, and the ~30 `const { a, b } = useXStore()` call sites in components/views. Low-risk, mechanical, immediately cuts re-renders.
3. **Migrate the six hand-rolled-localStorage stores to `persist` middleware.** `use-boards-bar-visibility-store`, `use-boards-filter-store`, `use-catalog-style-store`, `use-all-feed-filter-store`, `use-popular-threads-options-store`, `use-disclaimer-modal-store`. Use `migrate` to copy values from the legacy keys once. Deletes serialization code, unifies hydration semantics with the rest of the codebase, and removes a class of "default-true vs stored-false" bugs.
4. **Collapse the three near-identical modal stores (`use-directory-modal-store`, `use-create-board-modal-store`, `use-boards-bar-edit-modal-store`) into one `useModalsStore`** keyed by modal name. Merge `use-selected-text-store` into `use-reply-modal-store` (it has no other consumer). Net: -4 files, less cross-store wiring.
5. **Split `use-mod-queue-store` into persisted data vs session UI, and convert `getAlertThresholdSeconds` + `use-catalog-filters-store.getFilteredCountForCurrentCommunity` from actions to selectors.** Fixes stale-on-change reads in `post-desktop.tsx`, `post-mobile.tsx`, and `mod-queue.tsx`, and stops the `viewMode` toggle from invalidating every consumer of the 500-entry persisted history array.