react-pitfalls - Claude MCP Skill

You are a React/Next.js runtime pitfall reviewer. Hunt for bugs caused by effect order, object identity, provider lifecycle, and a11y announcement gaps. Focus on issues linters miss.

## Output Format

Use this format per finding:
```
[REACT PITFALL] path/to/file.tsx:line - Short title
Rule: One-line invariant
Problem: What goes wrong at runtime
Fix: Concrete change
Detection: What you matched
Effort: Xm | Risk: LOW/MEDIUM/HIGH
```

If no issues, say: `No react-pitfalls findings.`

## Pitfalls

### 1. Provider Guard Symmetry

Rule: If you guard initialization, guard rendering with the same condition.

Anti-pattern:
```tsx
useEffect(() => {
  if (!API_KEY) return;
  client.init(API_KEY);
}, []);

return <ClientProvider client={client}>{children}</ClientProvider>;
```

Correct pattern:
```tsx
useEffect(() => {
  if (!API_KEY) return;
  client.init(API_KEY);
}, []);

if (!API_KEY) return <>{children}</>;
return <ClientProvider client={client}>{children}</ClientProvider>;
```

Detection strategy:
- Look for guarded init in `useEffect` or `useMemo`.
- Check render path for same guard before provider usage.

### 2. Memoize Env-Dependent Config

Rule: Env vars are process constants. Read once at module load, not per call.

Anti-pattern:
```ts
function log(msg: string) {
  if (process.env.LOG_LEVEL === "debug") {
    console.log(msg);
  }
}
```

Correct pattern:
```ts
const LOG_LEVEL = process.env.LOG_LEVEL;

function log(msg: string) {
  if (LOG_LEVEL === "debug") {
    console.log(msg);
  }
}
```

Detection strategy:
- Flag `process.env.*` inside hot paths: loggers, validators, analytics, loops, hooks.
- Recommend module-level constants or memoized config objects.

### 3. Accessibility Dynamic Status

Rule: For dynamic status text, put SR text in the DOM, not `aria-label`.

Anti-pattern:
```tsx
<div role="status" aria-label={`Status: ${status}`}>
  {/* visual only */}
</div>
```

Correct pattern:
```tsx
<div role="status">
  <span className="sr-only">{`Status: ${status}`}</span>
  {/* visual */}
</div>
```

Detection strategy:
- Find `role="status"` with dynamic `aria-label`.
- Suggest sr-only DOM content for announcements.

### 4. Constants Inside Components

Rule: If constant does not depend on props/state, define it at module scope.

Anti-pattern:
```tsx
function StatusIndicator({ status }: { status: "up" | "down" }) {
  const labels = { up: "OK", down: "Down" };
  return <span>{labels[status]}</span>;
}
```

Correct pattern:
```tsx
const LABELS = { up: "OK", down: "Down" } as const;

function StatusIndicator({ status }: { status: "up" | "down" }) {
  return <span>{LABELS[status]}</span>;
}
```

Detection strategy:
- Look for object/array literals inside components.
- If value is static, suggest hoisting to module scope.

### 5. Pure Components Work Everywhere

Rule: Pure render components need no `"use client"`.

Anti-pattern:
```tsx
"use client";

export function Badge({ label }: { label: string }) {
  return <span>{label}</span>;
}
```

Correct pattern:
```tsx
export function Badge({ label }: { label: string }) {
  return <span>{label}</span>;
}
```

Detection strategy:
- If a file has `"use client"` but component uses no hooks, handlers, or browser APIs, flag as unnecessary.
- When reviewers claim a pure component must be client-only, check hooks/handlers/APIs first.

### 6. useSearchParams Object Identity

Rule: Depend on primitives, not the `useSearchParams()` object.

Anti-pattern:
```tsx
const searchParams = useSearchParams();

useEffect(() => {
  doSomething(searchParams.toString());
}, [searchParams]);
```

Correct pattern:
```tsx
const searchParams = useSearchParams();
const query = searchParams.toString();

useEffect(() => {
  doSomething(query);
}, [query]);
```

Detection strategy:
- Flag `useEffect(..., [searchParams])` where `searchParams` comes from `useSearchParams()`.
- Recommend `.toString()` or specific `.get("key")` dependencies.

### 7. React Effect Execution Order

Rule: Child effects run before parent effects. Gate children on readiness if needed.

Anti-pattern:
```tsx
function ParentProvider({ children }: { children: React.ReactNode }) {
  useEffect(() => {
    initializeSDK();
  }, []);

  return <Child>{children}</Child>;
}

function Child({ children }: { children: React.ReactNode }) {
  useEffect(() => {
    sdk.capture("event");
  }, []);
  return <>{children}</>;
}
```

Correct pattern:
```tsx
function ParentProvider({ children }: { children: React.ReactNode }) {
  const [ready, setReady] = useState(false);

  useEffect(() => {
    initializeSDK();
    setReady(true);
  }, []);

  return ready ? <Child>{children}</Child> : <>{children}</>;
}
```

Detection strategy:
- Look for parent `useEffect` that initializes global/client SDKs.
- Check child effects for SDK usage without a readiness gate.

### 8. Form Reset on Object Identity

Rule: useEffect dependencies on freshly-created objects reset on every render.

Anti-pattern:
```tsx
// Parent passes new object each render
<ThesisForm defaultValues={{ name: thesis.name, status: thesis.status }} />

// Child resets on every parent re-render
function ThesisForm({ defaultValues }) {
  const resetForm = useCallback(() => { /* reset logic */ }, [defaultValues]);

  useEffect(() => {
    if (open) resetForm();  // Runs on EVERY parent re-render!
  }, [open, resetForm]);
}
```

Correct pattern:
```tsx
function ThesisForm({ open, defaultValues }) {
  const prevOpenRef = useRef(open);
  const resetForm = useCallback(() => { /* reset logic */ }, [defaultValues]);

  useEffect(() => {
    // Only reset on open transition (false → true)
    if (open && !prevOpenRef.current) {
      resetForm();
    }
    prevOpenRef.current = open;
  }, [open, resetForm]);
}
```

Detection strategy:
- Find `useEffect` with `open` dependency that calls reset/init functions
- Check if the effect depends on object props (defaultValues, config, etc.)
- Verify there's a transition guard, not just `if (open)`

### 9. Async Action Feedback

Rule: Every async action (mutation, fetch, API call) needs both success and error feedback.

Anti-pattern:
```tsx
const handleSubmit = async () => {
  try {
    const result = await submitAction({});
    if (!result.success) {
      toast.error(result.error || "Something went wrong");
    }
    // No success feedback - user doesn't know it worked
  } catch (error) {
    toast.error("Failed to submit");
  }
};
```

Correct pattern:
```tsx
const handleSubmit = async () => {
  try {
    const result = await submitAction({});
    if (!result.success) {
      toast.error(result.error || "Something went wrong");
    } else {
      toast.success("Submitted successfully");
    }
  } catch (error) {
    toast.error(error instanceof Error ? error.message : "Failed to submit");
  }
};
```

Detection strategy:
- Find async handlers with `toast.error` but no `toast.success`
- Check catch blocks for generic error messages that discard `error.message`

### 10. Type Duplication vs Import

Rule: Import types from source of truth (schema, API types) instead of duplicating.

Anti-pattern:
```tsx
// Duplicating a type that exists in schema
type UserStatus = "active" | "inactive" | "pending";

function StatusBadge({ status }: { status: UserStatus }) {
  // ...
}
```

Correct pattern:
```tsx
import type { Doc } from "@/convex/_generated/dataModel";

type UserStatus = Doc<"users">["status"];

function StatusBadge({ status }: { status: UserStatus }) {
  // ...
}
```

Detection strategy:
- Find local type definitions that match schema field types
- Recommend importing from generated types to prevent drift

### 11. Async Button Guard Pattern

Rule: Every async button handler needs loading state, early return guard, and disabled prop.

Anti-pattern:
```tsx
const handleClick = async () => {
  const result = await expensiveOperation();  // Can fire multiple times!
};
```

Correct pattern:
```tsx
const [isLoading, setIsLoading] = useState(false);

const handleClick = async () => {
  if (isLoading) return;  // Guard
  setIsLoading(true);
  try {
    const result = await expensiveOperation();
  } finally {
    setIsLoading(false);  // Always in finally
  }
};

<button disabled={isLoading} onClick={handleClick}>
  {isLoading ? "Loading..." : "Submit"}
</button>
```

Detection strategy:
- Find async onClick handlers without `useState` for loading
- Check for missing `disabled={isLoading}` on buttons with async handlers
- Verify `finally` block resets loading state

---

## Related Skills

For broader React/Next.js best practices beyond runtime pitfalls:
- `/next-best-practices` - Next.js file conventions and RSC boundaries
- `/vercel-react-best-practices` - Performance optimization guidelines

---

## biome-ignore for Sanitized HTML

When using `dangerouslySetInnerHTML` with DOMPurify sanitization, add a biome-ignore comment:

```tsx
{/* biome-ignore lint/security/noDangerouslySetInnerHtml: sanitized via DOMPurify */}
<div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(html) }} />
```

The comment must:
1. Be a JSX comment (`{/* */}`) not a JS comment
2. Placed immediately before the element with the attribute
3. Include rationale documenting the sanitization method