Files
5chan/docs/agent-runs/codebase-audit-2026-04-23/01-react-anti-patterns.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

18 KiB

React Anti-Patterns Audit

Summary

Overall React health is moderate-to-good: the codebase mostly follows the repo's architectural rules (Zustand stores cover real shared state, Bitsocial hooks cover data fetching, Router is used for navigation). The anti-pattern concentration is in three places — a pervasive "derived-state via effect" style (scroll, title, keydown, resize), the two large Post components (post-desktop/post-mobile duplicate effect-heavy logic), and a handful of hook-level issues in use-directories, use-publish-reply, and the ModQueue subtree. There is almost no shared-state-in-useState anti-pattern left; the remaining useState calls are legitimately local UI state.

Findings

Critical

  • src/hooks/use-directories.ts:331-462useDirectories and useDirectoriesState are effectively the same hook duplicated wholesale (same useState + useEffect + module-cache + localStorage + GitHub fetch pattern, ~60 lines each). Both fall under the "no useEffect for data fetching; no copy-paste logic" rules, and both power virtually every view via useTheme, BoardLayout, Board, Catalog, Post, ModQueue, BoardsBar, etc. Fix: Collapse into a single Zustand directories store whose ensureHydrated() action is called once at app start (or from a thin shared wrapper hook). Remove the duplicated useState/useEffect bodies; keep useDirectories/useDirectoriesMetadata as pure selectors. This would also remove the awkward return cacheCommunities || state.communities || ... ternary on line 397.
  • src/views/mod-queue/mod-queue.tsx:672-772ModQueueCountItem is a "render-null-just-to-fire-an-effect-into-the-parent" pattern. Each feed item mounts, runs useEffect(() => onStatusChange(cid, { awaiting, urgent }), [...]) (line 682), and the parent ModQueueButtonContent aggregates a statusMap useState<Map> via handleStatusChange. Also has a stale-cleanup effect on line 710. This is textbook derived state pushed up via effects, and causes an O(feed) cascade of renders whenever anything in the feed changes. Fix: Compute normalCount/urgentCount during render in ModQueueButtonContent with a plain .reduce over feed (the awaiting/urgent predicates are pure functions of comment, currentTime, alertThresholdSeconds). Delete ModQueueCountItem, statusMap, handleStatusChange, and the cleanup effect.
  • src/components/reply-modal/reply-modal.tsx:104-288 — 8 useEffects in one component, several of which are orchestrating state that could be derived or moved into the existing useReplyModalStore/usePublishReply hooks. Notable: line 156 syncs parentCidRef.current.style.width from DOM measurement (fine), but line 163 mixes imperative textarea focus with an escape-key listener, line 186 is a dead one-shot useEffect(..., []) setting selection range on an empty textarea, line 196 imperatively mutates textRef.current.value (bypassing React) and calls setTimeout, line 226 guards an infinite loop via lastProcessedQuoteInsertRequestIdRef (see the comment on line 234 — the author acknowledged the effect fights itself). Fix: Split modal state into a useReplyModalController hook; model quote-insertion as an imperative handler triggered by store subscription (useEffect on a requestId is the smell), not an effect driven by changing store values.

High

  • src/components/post-desktop/post-desktop.tsx:930-1102 vs src/components/post-mobile/post-mobile.tsx:651-779 — post-desktop and post-mobile share three near-identical effect blocks: (1) setResetFunction when in post-page/pending view (desktop 930, mobile 651), (2) repliesResetRequestId reset guard using a ref (desktop 937, mobile 658), (3) Virtuoso scroll-state save listener (desktop 1089, mobile 767). Both files are ~1000+ LOC and most of the reply-rendering logic above these effects is also duplicated (filterRepliesForDisplay, directRepliesByParentCid, useQuotedByMap, useReplyHeightEstimates, useProgressiveRender, etc.). Fix: Extract a usePostReplies({ isMobile }) hook that owns the shared effects + memos, plus a useVirtuosoStateSave(key, enabled) hook for (3). The two components can keep only layout differences.
  • src/hooks/use-publish-reply.ts:135-208 vs src/hooks/use-publish-post.ts:86-98 — Both hooks use useState for publish-error/state-string that the child component (PostForm, ReplyModal) only reads back, plus an effect that resets those states when the publish inputs change. This is derived state synced by effect; the reset values are all null/false, and "when any publish input changes, error goes away" is a render-time computation using the previous error plus the latest input key. The two hooks also share the abandonPublishRef/publishOptionsWithAbandon/createBaseOptions/setPublishXxxOptions pattern almost verbatim (~80 LOC of duplication). Fix: Extract a usePublishLifecycle(store, { kind: 'post' | 'reply' }) helper; move the publishError reset into the publish action itself (on success/failure) instead of a dependency-array-driven effect. Consolidate the duplicate option-merging boilerplate.
  • src/components/boards-bar/boards-bar.tsx:31-62, 367-385SearchBar (desktop) has three separate effects for (a) focus on mount, (b) click-outside close, (c) escape-key close; BoardsBarMobile then repeats a setShowSearchBar pattern plus a scroll-listener useState(visible) + debounced-scroll-effect for nav hide/show. The click-outside + escape-to-close + focus-on-mount triplet is repeated across SettingsModal, CatalogFilters, ChallengeModal, ReplyModal, CreateBoardModal. Fix: Extract useEscapeToClose(onClose), useClickOutsideToClose(ref, onClose), useAutoFocus(ref) into src/hooks/. useScrollDirection() would replace the hand-rolled scroll listener + visible state.
  • src/views/board/board.tsx:313-413 — Seven useEffects in close proximity, most of which are route-sync effects guarded by isVisible. Several of them only derive pagination / infinite-scroll redirects from location.pathname + effectiveInfiniteScroll + totalPages (lines 378, 386, 396) — candidates for computing the navigation during the click handler or render rather than after mount. The "has been visible" scroll-to-top effect (line 549) uses a ref flag to simulate a once-per-mount guard; a key on the subtree or useNavigationType() at render would be cleaner. Document-title effect on line 579 duplicates the same pattern as in catalog.tsx:779 and post.tsx:333.
  • src/views/catalog/catalog.tsx:320-795 — Six useEffects, including a canonical-redirect effect (line 320) that duplicates the board.tsx:378 effect, a setCurrentCommunityAddress store-sync effect (line 339), a reset() triggered by filteredComments.length via a ref latch (line 513) — also duplicated at board.tsx:402 — a setResetFunction effect (line 522), and a doc-title effect (line 779). Fix: Extract useCanonicalMultiboardRedirect(location), useFeedResetFunction(reset, isVisible), and useReplayFeedReset(filteredComments, reset) hooks shared between board and catalog.
  • Document title + scroll-to-top duplication across views (src/views/blotter/blotter.tsx:15, src/views/faq/faq.tsx:7, src/views/pass/pass.tsx:32-38, src/views/rules/rules.tsx:181, src/views/pending-post/pending-post.tsx:19, src/views/home/home.tsx:193, src/views/archive/archive.tsx:208, src/views/board/board.tsx:579, src/views/catalog/catalog.tsx:779, src/views/post/post.tsx:333) — 10 views each implement their own useEffect(() => { document.title = ...; window.scrollTo(0, 0); }, [deps]). Fix: Add useDocumentTitle(title) and useScrollToTopOnMount() (or combined useViewInit({ title })) in src/hooks/ and replace.
  • src/components/catalog-search/catalog-search.tsx:19-26 — URL query param ?q= is the source of truth, but the effect on line 19 pushes queryParam into the useCatalogFiltersStore on every change. The component also holds a separate searchState useState and computes inputValue and openSearch by mixing URL, local state, and store. This is the "syncing derived state with an effect" + "boolean flag soup" smells combined. Fix: Make the store a selector of the URL (compute searchFilter from location.search during render, or subscribe via a single useSearchFilterFromUrl() selector); remove the effect. Model searchState as a discriminated { mode: 'closed' } | { mode: 'open'; value: string }.

Medium

  • src/hooks/use-fetch-gif-first-frame.ts:85-135useState + useEffect + module-level Set<string> for failed URLs + localForage cache. Classic "data fetching in an effect" that the repo rule says should live in a Bitsocial hook or a Zustand store. Also uses status strings ('idle'|'loading'|'ready'|'failed') which is fine, but the setGifFirstFrame((prev) => ...) identity checks (line 90, 95, 99, 126) are a code smell saying the effect is firing too often. Fix: Either (a) promote to a dedicated store indexed by URL with a thunk, or (b) turn into a useSyncExternalStore-backed cache so React subscribes rather than polls.
  • src/hooks/use-comment-media-info.ts:19-66 — Fetches thumbnail dimensions by constructing new Image() inside useEffect, stores the result in component-local useState, then re-derives CommentMediaInfo with the stored dimensions in a useMemo. This is "data fetch in effect" and the resulting thumbnailDimensions is local per-consumer — two components rendering the same link will both fetch and double-measure. Fix: Move the dimension cache to a module-level Map<string, { w, h }> or Zustand store keyed by link, driven by an async action invoked during render (suspense-style) or by a Bitsocial hook.
  • src/hooks/use-window-width.ts:3-15 + src/hooks/use-is-mobile.tsuseState + resize-listener useEffect. Fine in isolation, but every consumer subscribes independently (mobile vs desktop posts both call useWindowWidth, as do reply-height estimates, catalog, boards-bar). On resize, every subscriber rerenders. Fix: Back with useSyncExternalStore so React can bail out when the value hasn't changed, or keep the subscription in a tiny Zustand store so the read surface is selector-based.
  • src/views/account-data-editor/account-data-editor.tsx:52-78useState holds a single EditorState object with phase, AceEditor, aceOnBeforeLoad, and text. The effect on line 59 runs only when phase === 'loading' to import react-ace, then setEditorState replaces everything. This is a fine state machine, but handler functions mutate only text (line 141, 147) while phase is mutated via handleContinue/handleGoBack. The account dependency on line 78 will re-trigger the effect when account changes mid-load, which will double-import Ace. Fix: Split into phase + editor + text useStates or useReducer with explicit actions; guard the dynamic import with a ref so it only runs once per phase === 'loading' transition.
  • src/components/settings-modal/settings-modal.tsx:45-65expandedSections (a Set<string>) is local useState that shadows the URL hash, and an effect on line 60 re-syncs the set from hash with an explicit // eslint-disable-next-line react-hooks/exhaustive-deps. The disable comment is a red flag: the author silenced the lint rule because the effect is "wrong" by the rules but the fix is deeper. Fix: Derive expandedSections from hash plus a separate "user-added" set; or keep a single Zustand store for expanded sections so the hash is just the deep-linkable entry point.
  • src/views/post/post.tsx:293-297, 315-331, 381-429 — Seven effects on the PostPage component: /not-found redirect (line 293), "scroll to top unless thread-top intent" split across two effects (lines 315, 321) with consumedThreadTopScrollRef coordinating them, a cleanup-only resetThreadLiveUpdates effect (line 381), a thread-switch reset effect (line 387), and a refresh-by-cid orchestration effect (line 397-429). The split between the two scroll effects is explicitly documented in-file as subtle (comment at line 310) — a sign this is better modeled as one useLayoutEffect on [location.key] that reads the intent synchronously. Fix: Combine the two scroll effects into one keyed on location.key; move the refresh-by-cid orchestration into useThreadLiveUpdatesStore as an action invoked from a subscription in the store rather than an effect in the view.
  • src/components/reply-quote-preview/reply-quote-preview.tsx:133-148, 304-311 — Desktop and mobile variants each run their own window.addEventListener('resize', handleResize) effect; same pattern (and same comment style) in src/components/markdown/external-number-quote-link.tsx:83, and src/components/markdown/markdown.tsx:70. Fix: One useWindowResize(callback) hook.
  • src/components/edit-menu/edit-menu.tsx:45-110, 165-222 — Five useState slots plus defaultPublishEditOptions useMemo plus publishCommentEditOptions useState initialized from the memo. When defaultPublishEditOptions changes (lots of deps on line 92), the local useState is not reset — so opening the menu after the comment state changes can show stale options. Combined with banDuration (line 165) also initialized from defaultPublishEditOptions.commentModeration?.author?.banExpiresAt only once, this is a "state initialized from props and never re-synced" bug-magnet. Fix: Use a key={cid} wrapper or derive during render; for the editable slice of state that must be local, reset it in the "open" handler rather than by effect.

Low

  • src/components/markdown/external-number-quote-link.tsx:53-56 — Four correlated useStates (isResolving, isPreviewOpen, previewState, previewPosition). A discriminated previewState union ({ kind: 'idle' } | { kind: 'resolving' } | { kind: 'open'; position } | { kind: 'error' }) would eliminate impossible combinations and the setIsResolving(false) bookkeeping.
  • src/components/catalog-filters/catalog-filters.tsx:113-116, 240-252 — Two more escape-key-close effects; covered by the suggested useEscapeToClose extraction.
  • src/components/challenge-modal/challenge-modal.tsx:129-157, 232-305 — Four effects; line 129 sends a theme message post-load (fine), line 135 is a window-message handler (fine), line 232 and 297 are focus-on-mount and escape-to-close — again covered by the shared hooks extraction.
  • src/index.tsx:52-58window.history.back() inside the Capacitor back-button listener. Since the HashRouter is available at that point, this could be navigate(-1) via a small bridge, but the Capacitor listener is fired outside React tree, so calling window.history.back() directly is defensible. Fix: none required; keep the comment explicit and ignore from the "no manual history" rule since it's a native bridge, not in-app nav.
  • src/views/pass/pass.tsx:32-38 — Two adjacent useEffect(() => { ... }, []) blocks; trivially combinable into one. Covered by useDocumentTitle/useScrollToTopOnMount.
  • src/views/pending-post/pending-post.tsx:19, 28, 34 — Three effects, two of which are navigation side-effects. The post-driven redirect on line 34 fires on every post identity change; should depend on post?.cid + postCommunityAddress not on post as a whole.
  • src/components/boards-bar/boards-bar.tsx:140-142useBoardsBarVisibilityStore.getState().initialize() called from an empty-deps useEffect. If the store's initialize is idempotent this is fine, but it's an implicit "mount once, anywhere" that would be cleaner as a top-level store side-effect on creation.
  • src/components/post-form/post-form.tsx:391-476 — Five effects for display-name hydration, publish success navigation, reply success close, and upload callbacks — each fires once per success and sets a hasInitializedDisplayName ref to guard re-runs. The ref-gating pattern (also in reply-modal.tsx:282 and use-publish-reply.ts:147) is a known smell that says "this should be an event, not an effect".

Top 5 Actions

  1. Consolidate useDirectories / useDirectoriesState into a Zustand store. Removes ~100 LOC of duplicated fetch-in-effect, simplifies the "stable reference for memoization" contract that currently leaks into every consumer, and fixes the rule-violating data-fetch-in-effect at the root of the dependency tree (src/hooks/use-directories.ts:331-462).

  2. Delete the ModQueueCountItem / statusMap effect cascade. Compute normalCount and urgentCount purely during render in ModQueueButtonContent (src/views/mod-queue/mod-queue.tsx:672-772). This removes a per-feed-item render-null-to-fire-effect pattern that's both a textbook anti-pattern and a real performance hazard on boards with large queues.

  3. Create shared view-init hooks: useDocumentTitle, useScrollToTopOnMount, useEscapeToClose, useClickOutsideToClose, useAutoFocus, useWindowResize, useScrollDirection. Replace the ~20 scattered useEffects across views and modals (blotter, faq, pass, rules, home, archive, board, catalog, post, pending-post, plus boards-bar, settings-modal, catalog-filters, challenge-modal, reply-modal, reply-quote-preview, markdown, external-number-quote-link). High leverage, low risk — most replacements are mechanical.

  4. Refactor post-desktop / post-mobile shared logic into usePostReplies and useVirtuosoStateSave hooks. Both files are ~1000+ LOC, they duplicate reply filtering, height estimation, progressive rendering, fresh-replies registration, reset-function wiring, and Virtuoso scroll-state save listeners. Extracting the shared tail end (from freshRepliesForRender onward) into a hook cuts both files roughly in half and removes the parallel-edit risk.

  5. Split route-sync effects in board.tsx and catalog.tsx into shared hooks. Specifically: useCanonicalMultiboardRedirect, useFeedResetFunction(reset, isVisible), useReplayFeedResetOnAccountComments(filteredComments, reset), useScrollToTopOnFirstVisible(isVisible, navigationType). Both views currently each carry 6-9 near-duplicate effects; consolidating them reduces effect-count at the view layer and makes the remaining view code closer to pure render.