react-doctor

name	react-doctor
description	React/Next.js anti-pattern denetimi — state management, effects, re-render performansı, mimari sınırlar, client-side security ve accessibility eksikleri için yapılandırılmış audit. Mevcut bir React/Next.js yüzeyinde kod yazmadan ÖNCE veya /ulak-frontend war room'u sırasında "neyi yanlış yapıyoruz" sorusunu somut kontratlarla cevaplar. typescript-nextjs rule-pack'i frontend kalite-barında genişletir.
description_en	React/Next.js anti-pattern audit — structured review across state management, effects, re-render performance, architectural boundaries, client-side security, and accessibility. Surfaces "what are we doing wrong" with concrete contracts before touching code or during a /ulak-frontend war room. Extends the typescript-nextjs rule-pack on the frontend quality bar.
context	fork
agent	frontend-ios-flutter-director
allowed-tools	Read Grep Glob Bash

React Doctor — React/Next.js Anti-Pattern Audit

Why this exists

typescript-nextjs.md codifies the language + framework baseline (strict mode, no any, Server Components by default). It does NOT govern the runtime component contract: how state is owned, when effects are legitimate, why a tree re-renders, where the client/server boundary leaks, and whether the rendered DOM is operable by a keyboard or screen reader. Those failures pass tsc, pass lint, and ship — then surface as jank, hydration mismatches, XSS, and inaccessible flows in production.

This skill is the diagnostic layer between "it compiles" and "it is correct". It runs a six-axis audit (state / effects / performance / architecture / security / accessibility) and emits findings in the standard finding-schema.md shape so the director or /ulak-frontend can fold them into analysis-findings.md. It is read-only — it diagnoses, it does not refactor.

Attribution: audit category structure is adapted from react-doctor (millionco, MIT-licensed). Provenance recorded in NOTICE.md. No source text is reproduced; the contracts below are Ulak-native and map onto the existing anti-pattern catalogue.

When to use

• Before extending an existing React/Next.js surface (/ulak-frontend, EXTEND/REFACTOR mode) — diagnose before adding to the mess
• During Phase 2 parallel evidence on any repo whose runtime-manifest detects react + (next | client components)
• As a pre-merge gate on a frontend PR that touches state, effects, or data fetching
• NOT for greenfield scaffolds (saas-scaffolder prevents these by construction) and NOT for design/visual review (that is the design-system axis of /ulak-frontend)

Concrete contracts (six axes)

1. State management — owned, derived, or external; never duplicated

• Single source of truth. State that can be derived from props or other state MUST be derived inline, not mirrored into a second useState synced by an effect. The mirror-and-sync pair is the canonical React state bug.
• Server state is not client state. Data fetched from the server (lists, entities, sessions) lives in a server-cache layer (RSC fetch, React Query, SWR) — not hand-rolled useState + useEffect fetch. Hand-rolled fetch loses dedup, retry, stale-while-revalidate, and request-cancellation for free.
• Lift only as far as needed. State lifted above its consumers forces sibling re-renders; state pushed into the wrong provider makes the whole subtree a re-render victim. Owner = lowest common ancestor of all readers.
• No prop-drilling past 3 levels for cross-cutting concerns (theme, locale, auth) — use context; but context value MUST be memoized or every consumer re-renders on every parent render.

2. Effects — synchronize with the outside world, nothing else

• useEffect is for external systems, not for reacting to state to compute more state. Transforming data on render is a render-time calculation, not an effect.
• Banned: effect-as-derived-state. useEffect(() => setFullName(first + ' ' + last), [first, last]) is a derived value (const fullName = first + ' ' + last), not an effect. It double-renders and can desync.
• Banned: empty/wrong dependency arrays that mask the lint warning instead of fixing the closure. A missing dep is a stale-closure bug; a disabled exhaustive-deps line needs a comment justifying it.
• Cleanup is mandatory for subscriptions, timers, listeners, and abortable fetches. An effect that subscribes without returning a teardown leaks on every re-run and on unmount.
• Data fetching in an effect on the client for first paint is a typescript-nextjs.md violation — fetch in the Server Component or route.ts.

3. Performance — re-render is the default failure mode

• Identity-stable props. Inline object/array/function literals (<C style={{}} onClick={() => …}>) create a new reference every render, defeating memo on the child. Hoist or useMemo/useCallback when the child is memoized or the value seeds a dependency array.
• key is identity, not index. List key={index} breaks reconciliation on insert/reorder/delete — state attaches to the wrong row. Keys come from stable entity IDs.
• No expensive work on render path without useMemo (sorting/filtering large lists, parsing, formatting). Profile before memoizing trivial work — premature useMemo is also a smell.
• Context split by change frequency. A single context holding both rarely-changing config and fast-changing UI state re-renders every consumer on the fast change. Split into stable + volatile contexts.
• Suspense + streaming over blocking waterfalls. Sequential dependent fetches that block paint are a waterfall; parallelize or stream with boundaries.

4. Architecture — boundaries, not kitchen-sink components

• Component does one thing. A component that fetches, transforms, manages 6 pieces of state, and renders 300 lines is a god-component — extract data hooks, presentational children, and a container.
• Client/server boundary is explicit and minimal. "use client" marks the smallest island that needs interactivity. A "use client" at the page root drags the entire tree to the client and forfeits RSC. Push the directive down to the leaf.
• No business logic in components. Pricing, permissions, validation rules live in lib/, are unit-testable, and are imported — not inlined in JSX.
• Feature-organized, not type-organized beyond a shared ui/ primitive layer (aligns with saas-scaffolder component structure).

5. Security — the client is hostile territory

• Banned: dangerouslySetInnerHTML with unsanitized input. Any HTML from user content, CMS, or API MUST pass through dompurify (or be server-sanitized) before injection — stored/reflected XSS vector.
• No secrets in client bundles. Only NEXT_PUBLIC_* env vars reach the browser; a secret key referenced in a Client Component ships to every visitor. Server-role keys stay behind import 'server-only' (AP-13).
• href/src from data is validated. A javascript: URL in a user-supplied link is script execution; an open next/router push from a ?next= param is open-redirect (security-primitive-fail-closed.md §Redirect).
• Auth/role checks are not client-only. A hidden admin button is UX, not security — the gate lives on the server route. Client-side route guards are bypassable.
• Trusted-Types / CSP friendliness. No inline event handlers injected as strings; align with the CSP contract in security-primitive-fail-closed.md §CSP.

6. Accessibility — the rendered DOM must be operable

• Semantic elements first. <button> for actions, <a href> for navigation — never a <div onClick> that traps keyboard and screen-reader users. Interactive <div> requires role + tabIndex + key handlers, which is strictly worse than the native element.
• Every input has a programmatic label. <label htmlFor> or aria-label; placeholder is not a label.
• Focus is managed across route changes, modal open/close, and async content swaps. A modal that opens without trapping focus and restoring it on close is unusable with a keyboard.
• Images and icon-buttons have text alternatives. alt on <img> (empty alt="" for decorative); aria-label on icon-only buttons.
• Color is not the only signal, hit targets ≥24px, and :focus-visible is never removed without a replacement indicator (defer richer visual/contrast review to /ulak-frontend design-system axis).

Anti-patterns this skill flags (mapped to catalogue)

Code	Axis	Symptom
AP-RD-01	State	Derived value mirrored into useState + synced by effect
AP-RD-02	State	Server data hand-fetched into useState/useEffect instead of a cache layer
AP-RD-03	Effects	useEffect used to compute state from state (effect-as-derived)
AP-RD-04	Effects	Subscription/timer/listener without cleanup; stale-closure via wrong deps
AP-RD-05	Performance	key={index} on a mutable list; inline literals defeating memo
AP-RD-06	Performance	Single context mixing stable + volatile state; render waterfall
AP-RD-07	Architecture	"use client" at page root; god-component; business logic in JSX
AP-RD-08	Security	dangerouslySetInnerHTML unsanitized; secret in client bundle; javascript:/open-redirect href
AP-RD-09	A11y	<div onClick> action; unlabeled input; unmanaged focus; missing alt

These extend the existing catalogue; AP-RD-08 cross-references AP-13 (server-only), AP-47 (open-redirect), AP-48 (XSS) in anti-patterns.md.

Process

1. Read runtime-manifest.md — confirm react/next + locate component, hook, and app/ directories with file:line precision.
2. Grep the six axes (examples below) and read the hits — a grep hit is a candidate, confirmed only by reading the surrounding component.
3. Emit one finding-schema.md entry per confirmed issue with axis, AP-RD code, file:line, T1/T2 evidence, and a one-line fix direction.
4. Append a did-you-know note for non-obvious findings (e.g. context not memoized, "use client" higher than necessary, effect that should be render-time).
5. Hand the findings to the director / /ulak-frontend — this skill does not edit code.

CI validators (referenced, not owned here)

• scripts/validate-no-effect-derived-state.sh — flags useEffect whose body is solely setX(...) over render-scope values (effect-as-derived candidate)
• scripts/validate-no-index-keys.sh — grep for key={index} / key={i} in .map( callbacks on app components
• scripts/validate-use-client-depth.sh — reports "use client" files that are route entrypoints (push-down candidates)
• scripts/validate-no-dangerous-html-unsanitized.sh — dangerouslySetInnerHTML not preceded by a sanitizer import in the same module
• scripts/validate-no-public-secret.sh — non-NEXT_PUBLIC_ env reference inside a "use client" module
• scripts/validate-interactive-semantics.sh — onClick on <div>/<span> without role + tabIndex + key handler
• scripts/validate-img-alt.sh — <img> / next/image without an alt attribute (empty allowed for decorative)

These are advisory diagnostics, not deploy-blocking gates by default; a project promotes any of them to CI-blocking in .claude/rules/react.md.

Collision rule

Project .claude/rules/react.md overrides specific axis contracts (a project may document a reason for a specific dangerouslySetInnerHTML with its own sanitizer, or an intentional non-memoized context); unmatched contracts inherit. This skill does NOT replace typescript-nextjs.md — it layers the component-runtime + a11y + client-security audit atop the language/framework baseline.

Integration

• docs/runtime/rule-packs/typescript-nextjs.md — language + framework baseline this skill diagnoses against
• docs/runtime/rule-packs/security-primitive-fail-closed.md — §CSP and §Redirect referenced by axis 5
• docs/runtime/rule-packs/i18n-routing-discipline.md — raw-anchor / locale-link discipline overlaps axis 6 navigation
• docs/runtime/anti-patterns.md — AP-13, AP-47, AP-48 referenced; AP-RD-01..09 extend the catalogue
• docs/governance/finding-schema.md — output finding shape
• .claude/commands/ulak-frontend.md — the war-room command that dispatches this skill alongside design-system + a11y review
• .claude/skills/saas-scaffolder/SKILL.md — prevents these by construction in greenfield; this skill is the brownfield counterpart
• NOTICE.md — react-doctor (millionco, MIT) attribution

Canonical footer

Authoritative as of Ulak OS v2.3.0. Audit category structure adapted from react-doctor (millionco, MIT) — see NOTICE.md; all contracts are Ulak-native and map onto the existing anti-pattern catalogue. This skill is the brownfield/EXTEND counterpart to saas-scaffolder (which prevents these by construction) and the component-runtime sibling of the typescript-nextjs rule-pack (which governs language/framework baseline only). Dispatched by /ulak-frontend; consumable directly during Phase 2 parallel evidence on any React/Next.js surface.