Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions .agents/skills/review-react/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
name: review-react
description: >
React code review guidelines covering Rules of React, re-render optimization,
rendering performance, and advanced patterns. Activates when writing, reviewing,
or refactoring React components, hooks, or state management code.
---

# React Code Review Guidelines

Performance optimization and correctness guide for React applications. Contains 23 rules across 4 categories, prioritized by impact.

## When to Apply

Reference these guidelines when:
- Writing or reviewing React components and hooks
- Optimizing re-render performance
- Refactoring state management or effect logic
- Reviewing pull requests that touch React code

## Rule Categories by Priority

| Priority | Category | Impact | Prefix | Rules |
|----------|----------|--------|--------|-------|
| 1 | Rules of React | CRITICAL | `react-rules-` | 3 |
| 2 | Re-render Optimization | MEDIUM | `rerender-` | 13 |
| 3 | Rendering Performance | MEDIUM | `rendering-` | 5 |
| 4 | Advanced Patterns | LOW | `advanced-` | 2 |

## Quick Reference

### 1. Rules of React (CRITICAL)

- `react-rules-purity` - Components and Hooks must be pure; no side effects during render
- `react-rules-hooks` - Only call Hooks at the top level and from React functions
- `react-rules-calling` - Never call components as functions or pass Hooks as values

### 2. Re-render Optimization (MEDIUM)

- `rerender-no-inline-components` - Never define components inside other components
- `rerender-derived-state-no-effect` - Derive state during render, not in effects
- `rerender-memo` - Extract memoized child components to avoid re-renders
- `rerender-memo-with-default-value` - Hoist default non-primitive props outside memo
- `rerender-simple-expression-in-memo` - Don't useMemo for simple primitive expressions
- `rerender-defer-reads` - Don't subscribe to state only used in callbacks
- `rerender-dependencies` - Use primitive values in effect dependencies
- `rerender-derived-state` - Subscribe to derived booleans, not raw objects
- `rerender-functional-setstate` - Use functional setState for stable callbacks
- `rerender-lazy-state-init` - Pass initializer function to useState for expensive values
- `rerender-move-effect-to-event` - Move interaction logic from effects to event handlers
- `rerender-transitions` - Use startTransition for non-urgent state updates
- `rerender-use-ref-transient-values` - Use refs for frequently-changing transient values

### 3. Rendering Performance (MEDIUM)

- `rendering-hoist-jsx` - Hoist static JSX outside component functions
- `rendering-conditional-render` - Use ternary operator instead of && for conditional rendering
- `rendering-usetransition-loading` - Prefer useTransition over manual loading state
- `rendering-content-visibility` - Use CSS content-visibility: auto for long lists
- `rendering-activity` - Use Activity component for preserving hidden UI state

### 4. Advanced Patterns (LOW)

- `advanced-event-handler-refs` - Store latest event handlers in refs for stable callbacks
- `advanced-init-once` - Initialize app-level singletons once, not per mount

## How to Use

Read individual rule files for detailed explanations and code examples:

```
rules/react-rules-purity.md
rules/rerender-no-inline-components.md
```

Each rule file contains:
- Brief explanation of why it matters
- Incorrect code example
- Correct code example
- Reference links
26 changes: 26 additions & 0 deletions .agents/skills/review-react/rules/_sections.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Sections

This file defines all sections, their ordering, impact levels, and descriptions.
The section ID (in parentheses) is the filename prefix used to group rules.

---

## 1. Rules of React (react-rules)

**Impact:** CRITICAL
**Description:** Fundamental rules from react.dev that ensure correctness. Components must be pure, Hooks must follow call rules, and components must not be called as functions.

## 2. Re-render Optimization (rerender)

**Impact:** MEDIUM
**Description:** Patterns to minimize unnecessary re-renders: proper memoization, derived state, functional setState, and effect dependency management.

## 3. Rendering Performance (rendering)

**Impact:** MEDIUM
**Description:** Techniques to optimize what and how React renders: hoisting static JSX, conditional rendering patterns, content-visibility, and transitions.

## 4. Advanced Patterns (advanced)

**Impact:** LOW
**Description:** Specialized techniques for edge cases: storing handlers in refs for stable callbacks and one-time initialization patterns.
28 changes: 28 additions & 0 deletions .agents/skills/review-react/rules/_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
title: Rule Title Here
impact: MEDIUM
impactDescription: Optional description of impact (e.g., "20-50% improvement")
tags: tag1, tag2
---

## Rule Title Here

**Impact: MEDIUM (optional impact description)**

Brief explanation of the rule and why it matters. This should be clear and concise, explaining the performance implications.

**Incorrect (description of what's wrong):**

```typescript
// Bad code example here
const bad = example()
```

**Correct (description of what's right):**

```typescript
// Good code example here
const good = example()
```

Reference: [Link to documentation or resource](https://example.com)
55 changes: 55 additions & 0 deletions .agents/skills/review-react/rules/advanced-event-handler-refs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
title: Store Event Handlers in Refs
impact: LOW
impactDescription: stable subscriptions
tags: advanced, hooks, refs, event-handlers, optimization
---

## Store Event Handlers in Refs

Store callbacks in refs when used in effects that shouldn't re-subscribe on callback changes.

**Incorrect (re-subscribes on every render):**

```tsx
function useWindowEvent(event: string, handler: (e) => void) {
useEffect(() => {
window.addEventListener(event, handler)
return () => window.removeEventListener(event, handler)
}, [event, handler])
}
```

**Correct (stable subscription):**

```tsx
function useWindowEvent(event: string, handler: (e) => void) {
const handlerRef = useRef(handler)
useEffect(() => {
handlerRef.current = handler
}, [handler])

useEffect(() => {
const listener = (e) => handlerRef.current(e)
window.addEventListener(event, listener)
return () => window.removeEventListener(event, listener)
}, [event])
}
```

**Alternative: use `useEffectEvent` if you're on latest React:**

```tsx
import { useEffectEvent } from 'react'

function useWindowEvent(event: string, handler: (e) => void) {
const onEvent = useEffectEvent(handler)

useEffect(() => {
window.addEventListener(event, onEvent)
return () => window.removeEventListener(event, onEvent)
}, [event])
}
```

`useEffectEvent` provides a cleaner API for the same pattern: it creates a stable function reference that always calls the latest version of the handler.
42 changes: 42 additions & 0 deletions .agents/skills/review-react/rules/advanced-init-once.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
title: Initialize App Once, Not Per Mount
impact: LOW-MEDIUM
impactDescription: avoids duplicate init in development
tags: initialization, useEffect, app-startup, side-effects
---

## Initialize App Once, Not Per Mount

Do not put app-wide initialization that must run once per app load inside `useEffect([])` of a component. Components can remount and effects will re-run. Use a module-level guard or top-level init in the entry module instead.

**Incorrect (runs twice in dev, re-runs on remount):**

```tsx
function Comp() {
useEffect(() => {
loadFromStorage()
checkAuthToken()
}, [])

// ...
}
```

**Correct (once per app load):**

```tsx
let didInit = false

function Comp() {
useEffect(() => {
if (didInit) return
didInit = true
loadFromStorage()
checkAuthToken()
}, [])

// ...
}
```

Reference: [Initializing the application](https://react.dev/learn/you-might-not-need-an-effect#initializing-the-application)
66 changes: 66 additions & 0 deletions .agents/skills/review-react/rules/react-rules-calling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
---
title: React Calls Components and Hooks
impact: CRITICAL
impactDescription: breaks React's rendering model and optimization
tags: react-rules, components, hooks, calling-convention
---

## React Calls Components and Hooks

**Impact: CRITICAL (breaks React's rendering model and optimization)**

React must control when components render and hooks execute. Calling them directly bypasses reconciliation, state management, and optimization.

### Rule 1: Never call component functions directly

**Incorrect (calling component as function):**

```tsx
function Parent() {
// This bypasses React's rendering, no proper lifecycle or state isolation
return <div>{Profile({ name: 'Alice' })}</div>
}
```

**Correct (use JSX):**

```tsx
function Parent() {
return <div><Profile name="Alice" /></div>
}
```

Calling a component as a function makes React treat it as part of the parent's render. This means:
- No separate fiber node for reconciliation
- State and effects are tied to the parent
- Keys and refs don't work as expected

### Rule 2: Never pass Hooks as regular values

**Incorrect (passing hook as prop):**

```tsx
function ChatRoom({ useStatus }) {
const status = useStatus() // Hook passed as value
return <p>{status}</p>
}

<ChatRoom useStatus={useOnlineStatus} />
```

**Correct (call hook directly, pass result as prop):**

```tsx
function ChatRoom({ status }) {
return <p>{status}</p>
}

function ChatRoomWrapper() {
const status = useOnlineStatus()
return <ChatRoom status={status} />
}
```

Passing hooks as values makes them opaque to React's static analysis, breaks the Rules of Hooks, and prevents the compiler from optimizing correctly.

Reference: [React calls Components and Hooks](https://react.dev/reference/rules/react-calls-components-and-hooks)
91 changes: 91 additions & 0 deletions .agents/skills/review-react/rules/react-rules-hooks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
title: Rules of Hooks
impact: CRITICAL
impactDescription: violating causes runtime errors and broken state
tags: react-rules, hooks, top-level
---

## Rules of Hooks

**Impact: CRITICAL (violating causes runtime errors and broken state)**

Hooks rely on a stable call order. Calling them conditionally or in loops breaks React's ability to track state correctly.

### Rule 1: Only call Hooks at the top level

**Incorrect (hook inside condition):**

```tsx
function Form({ showName }) {
if (showName) {
const [name, setName] = useState('') // Conditional hook call
}
const [email, setEmail] = useState('')
return <input value={email} onChange={e => setEmail(e.target.value)} />
}
```

**Correct (always call hooks, conditionally use values):**

```tsx
function Form({ showName }) {
const [name, setName] = useState('')
const [email, setEmail] = useState('')
return (
<>
{showName && <input value={name} onChange={e => setName(e.target.value)} />}
<input value={email} onChange={e => setEmail(e.target.value)} />
</>
)
}
```

**Incorrect (hook inside loop):**

```tsx
function Filters({ filters }) {
const values = []
for (const f of filters) {
values.push(useState(f.default)) // Hook in loop
}
return <>{/* ... */}</>
}
```

**Correct (extract to child component or use single state):**

```tsx
function Filters({ filters }) {
const [values, setValues] = useState(() =>
Object.fromEntries(filters.map(f => [f.id, f.default]))
)
return <>{/* ... */}</>
}
```

### Rule 2: Only call Hooks from React functions

**Incorrect (hook in regular function):**

```tsx
function getUser() {
const [user, setUser] = useState(null) // Not a React component
return user
}
```

**Correct (hook in component or custom hook):**

```tsx
function useUser() {
const [user, setUser] = useState(null)
return user
}

function Profile() {
const user = useUser()
return <p>{user?.name}</p>
}
```

Reference: [Rules of Hooks](https://react.dev/reference/rules/rules-of-hooks)
Loading
Loading