feat(agents): add React code review skill for agent-based guidance#687
feat(agents): add React code review skill for agent-based guidance#687
Conversation
Add comprehensive React code review guidelines as an Agent Skill following the vercel-labs/agent-skills standard. Includes 23 rules across 4 categories: Rules of React, Re-render Optimization, Rendering Performance, and Advanced Patterns. Skill is available in .agents/skills/review-react and symlinked to .claude/skills for Claude Code auto-discovery. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | d1e104f | Commit Preview URL | Mar 17 2026, 10:12 AM |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis pull request adds a comprehensive React code review skill system, including a main skill definition, four categorized rule sections (React Rules, Re-render Optimization, Rendering Performance, Advanced Patterns), rule templates, and 20+ individual documentation files covering React best practices. The skill is integrated via pointer references in agent configuration files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.agents/skills/review-react/rules/rerender-dependencies.md (1)
12-13: Tighten the wording to avoid an absolute claim.“Object and array dependencies create new references each render” is not always true; they often change identity when recreated. Consider softening to “can create new references” for technical precision.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/review-react/rules/rerender-dependencies.md around lines 12 - 13, Update the sentence "Object and array dependencies create new references each render, causing effects to re-run unnecessarily." to a softened, more precise phrasing such as "Object and array dependencies can create new references when recreated each render, which may cause effects to re-run unnecessarily," and ensure the guidance still recommends extracting the primitive values you actually depend on..agents/skills/review-react/rules/rerender-lazy-state-init.md (1)
24-31: Consider clarifying initialization semantics.The lazy initializer example correctly demonstrates the pattern. However, it might be worth noting that the initializer only runs once on mount and won't reflect changes to the
contentprop. This is the intended behavior for initialization, but could be clarified for developers who might expect it to update when props change.Optional clarification
**Correct (lazy initializer runs only on first render):** ```tsx function Editor({ content }) { + // Runs once on mount; won't update if content prop changes const [parsed, setParsed] = useState(() => parseMarkdown(content)) return <Preview data={parsed} /> }</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.agents/skills/review-react/rules/rerender-lazy-state-init.md around lines
24 - 31, Add a short inline comment above the useState lazy initializer in the
Editor component to clarify that useState(() => parseMarkdown(content)) runs
only once on mount and will not re-run when the content prop changes; reference
the Editor function, the parsed state variable, the useState call, and the
parseMarkdown(content) initializer so reviewers understand the intended
initialization semantics and avoid expecting automatic updates from prop
changes.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/review-react/rules/advanced-init-once.md:
- Around line 27-46: The module-level singleton init pattern using the
initialized flag and initApp() is unsafe for SSR because module-scoped mutable
state can persist across requests; update the docs/examples to explicitly warn
that the pattern is for client bootstrap only and add a prominent caveat near
initApp()/App that this must not be used in server/SSR code (mention initialized
and initApp by name) and suggest alternative approaches for SSR (per-request
initialization or request-scoped lifecycles) so readers don't apply it in
Node/SSR contexts.In @.agents/skills/review-react/rules/rendering-activity.md:
- Around line 31-51: Update the example and docs to use the stable Activity
export instead of the outdated unstable_Activity: change the import to import {
Activity } from 'react', update the example component (Tabs) to reference
Activity (no unstable_ prefix), remove the "unstable" note about React 19 and
its API possibly changing, and replace the RFC link with the current React
docs/API reference for Activity so developers are pointed to the stable
documentation.In @.agents/skills/review-react/rules/rendering-usetransition-loading.md:
- Around line 21-25: The example's switchTab function is incorrectly marked
async but contains no awaited work, so setLoading(false) runs immediately and
doesn't model a real async loading state; update the switchTab implementation
(referencing switchTab, setLoading, setTab) to either remove the async keyword
if no async work is intended, or include an actual awaited operation (e.g.,
awaiting a fetch or a short delay promise) between setLoading(true) and
setLoading(false) so the loading state spans the async work and correctly
demonstrates the issue.In @.agents/skills/review-react/rules/rerender-defer-reads.md:
- Around line 12-34: The "incorrect" example contradicts its premise because the
Form component binds draft in JSX; update the snippet so draft is truly only
read in callbacks by removing the value={draft} binding on the input (leaving
uncontrolled input or using a ref) and adjust the explanation to state that
draft is only accessed inside handleAdd (and possibly via a ref), or
alternatively reword the rule text to reflect that draft is currently used in
JSX; target symbols: Form, draft, handleAdd, ItemList.
Nitpick comments:
In @.agents/skills/review-react/rules/rerender-dependencies.md:
- Around line 12-13: Update the sentence "Object and array dependencies create
new references each render, causing effects to re-run unnecessarily." to a
softened, more precise phrasing such as "Object and array dependencies can
create new references when recreated each render, which may cause effects to
re-run unnecessarily," and ensure the guidance still recommends extracting the
primitive values you actually depend on.In @.agents/skills/review-react/rules/rerender-lazy-state-init.md:
- Around line 24-31: Add a short inline comment above the useState lazy
initializer in the Editor component to clarify that useState(() =>
parseMarkdown(content)) runs only once on mount and will not re-run when the
content prop changes; reference the Editor function, the parsed state variable,
the useState call, and the parseMarkdown(content) initializer so reviewers
understand the intended initialization semantics and avoid expecting automatic
updates from prop changes.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `abab1838-fb02-4dc5-9e69-504349d86068` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c740c5e28c30afc23fff8705c405cefa514a1f35 and d1e104f043af1a6cedf4729f15dd032b5a0c4215. </details> <details> <summary>📒 Files selected for processing (28)</summary> * `.agents/skills/review-react/SKILL.md` * `.agents/skills/review-react/rules/_sections.md` * `.agents/skills/review-react/rules/_template.md` * `.agents/skills/review-react/rules/advanced-event-handler-refs.md` * `.agents/skills/review-react/rules/advanced-init-once.md` * `.agents/skills/review-react/rules/react-rules-calling.md` * `.agents/skills/review-react/rules/react-rules-hooks.md` * `.agents/skills/review-react/rules/react-rules-purity.md` * `.agents/skills/review-react/rules/rendering-activity.md` * `.agents/skills/review-react/rules/rendering-conditional-render.md` * `.agents/skills/review-react/rules/rendering-content-visibility.md` * `.agents/skills/review-react/rules/rendering-hoist-jsx.md` * `.agents/skills/review-react/rules/rendering-usetransition-loading.md` * `.agents/skills/review-react/rules/rerender-defer-reads.md` * `.agents/skills/review-react/rules/rerender-dependencies.md` * `.agents/skills/review-react/rules/rerender-derived-state-no-effect.md` * `.agents/skills/review-react/rules/rerender-derived-state.md` * `.agents/skills/review-react/rules/rerender-functional-setstate.md` * `.agents/skills/review-react/rules/rerender-lazy-state-init.md` * `.agents/skills/review-react/rules/rerender-memo-with-default-value.md` * `.agents/skills/review-react/rules/rerender-memo.md` * `.agents/skills/review-react/rules/rerender-move-effect-to-event.md` * `.agents/skills/review-react/rules/rerender-no-inline-components.md` * `.agents/skills/review-react/rules/rerender-simple-expression-in-memo.md` * `.agents/skills/review-react/rules/rerender-transitions.md` * `.agents/skills/review-react/rules/rerender-use-ref-transient-values.md` * `.claude/skills/review-react` * `AGENTS.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| **Correct (module-level initialization):** | ||
|
|
||
| ```tsx | ||
| // app-init.ts | ||
| let initialized = false | ||
|
|
||
| export function initApp() { | ||
| if (initialized) return | ||
| initialized = true | ||
| analytics.init('key') | ||
| errorTracker.setup() | ||
| } | ||
| ``` | ||
|
|
||
| ```tsx | ||
| // App.tsx | ||
| import { initApp } from './app-init' | ||
|
|
||
| initApp() // Runs once at module evaluation | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ".agents/skills/review-react/rules/advanced-init-once.md"Repository: daangn/stackflow
Length of output: 2072
Add SSR/runtime-scope caveat for module-level singleton init pattern.
The module-level initialization pattern shown at lines 27–46 lacks a critical caveat: module-scoped mutable state (the initialized flag) persists across module evaluations and can cause cross-request contamination in server-side rendering contexts. Readers working on SSR or Node.js server modules could inadvertently apply this pattern in shared code and skip initialization for subsequent requests.
Scope this guidance explicitly to client bootstrap code only, or add a prominent warning against server/SSR usage.
Suggested doc patch
-**Correct (module-level initialization):**
+**Correct (module-level initialization in client bootstrap only):**
```tsx
// app-init.ts
@@ -42,6 +42,8 @@ export function initApp() {
initApp() // Runs once at module evaluation
+// ⚠️ Use only in client browser context. In SSR, this pattern risks skipping initialization for subsequent requests.
function App() {
return <Router />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/review-react/rules/advanced-init-once.md around lines 27 -
46, The module-level singleton init pattern using the initialized flag and
initApp() is unsafe for SSR because module-scoped mutable state can persist
across requests; update the docs/examples to explicitly warn that the pattern is
for client bootstrap only and add a prominent caveat near initApp()/App that
this must not be used in server/SSR code (mention initialized and initApp by
name) and suggest alternative approaches for SSR (per-request initialization or
request-scoped lifecycles) so readers don't apply it in Node/SSR contexts.
| import { unstable_Activity as Activity } from 'react' | ||
|
|
||
| function Tabs({ activeTab }) { | ||
| return ( | ||
| <div> | ||
| <Activity mode={activeTab === 'home' ? 'visible' : 'hidden'}> | ||
| <HomeTab /> | ||
| </Activity> | ||
| <Activity mode={activeTab === 'profile' ? 'visible' : 'hidden'}> | ||
| <ProfileTab /> | ||
| </Activity> | ||
| </div> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| When `mode="hidden"`, the component tree is rendered at lower priority and hidden via CSS. Effects are cleaned up when hidden and re-run when shown. | ||
|
|
||
| Note: `Activity` is currently `unstable_Activity` in React 19. The API may change. | ||
|
|
||
| Reference: [Activity (React RFC)](https://github.com/reactjs/rfcs/blob/main/text/0extracting-state-updates-from-the-rendering-process.md) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "rendering-activity.md" -type fRepository: daangn/stackflow
Length of output: 117
🏁 Script executed:
cat -n .agents/skills/review-react/rules/rendering-activity.md 2>/dev/null | head -60Repository: daangn/stackflow
Length of output: 2015
🌐 Web query:
React Activity API current documentation React 19 2024
💡 Result:
Official, current docs for React’s Activity API (React 19)
<Activity>reference (react@19.2):react.dev/reference/react/Activity— describes the component, itsmode: 'visible' | 'hidden', and how it hides UI withdisplay: none, preserves state/DOM, and cleans up/recreates Effects when hidden/reshown. [1]- Background + SSR notes and rationale (React Labs post):
react.dev/blog/2025/04/23/react-labs-view-transitions-activity-and-more— includes guidance like SSR behavior whenmode="hidden"and hydration prioritization whenmode="visible". [2] - React 19 announcement (Dec 5, 2024):
react.dev/blog/2024/12/05/react-19— the main React 19 release post (Activity isn’t the focus there; Activity is documented under the 19.2 reference). [3]
Sources:
[1] (react.dev)
[2] (react.dev)
[3] (react.dev)
Citations:
- 1: https://react.dev/reference/react/Activity
- 2: https://react.dev/blog/2025/04/23/react-labs-view-transitions-activity-and-more
- 3: https://react.dev/blog/2024/12/05/react-19
Update Activity API import and documentation (outdated unstable reference).
Lines 31, 49, and 51 reference unstable_Activity and an RFC, but current React docs (19.2+) export stable Activity from react. This guidance will mislead developers to use outdated imports. Update the import statement, remove the instability note, and point to the current API reference.
Suggested doc patch
-import { unstable_Activity as Activity } from 'react'
+import { Activity } from 'react'
@@
-Note: `Activity` is currently `unstable_Activity` in React 19. The API may change.
+
@@
-Reference: [Activity (React RFC)](https://github.com/reactjs/rfcs/blob/main/text/0extracting-state-updates-from-the-rendering-process.md)
+Reference: [Activity](https://react.dev/reference/react/Activity)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/review-react/rules/rendering-activity.md around lines 31 -
51, Update the example and docs to use the stable Activity export instead of the
outdated unstable_Activity: change the import to import { Activity } from
'react', update the example component (Tabs) to reference Activity (no unstable_
prefix), remove the "unstable" note about React 19 and its API possibly
changing, and replace the RFC link with the current React docs/API reference for
Activity so developers are pointed to the stable documentation.
| const switchTab = async (newTab: string) => { | ||
| setLoading(true) | ||
| setTab(newTab) | ||
| setLoading(false) // Flash of loading state | ||
| } |
There was a problem hiding this comment.
Fix the misleading async example.
The "Incorrect" example has a logical issue: the function is marked async but contains no await statement, and setLoading(false) is called synchronously immediately after setTab(newTab). This doesn't accurately demonstrate the problem with manual loading states—there's no async operation for the loading state to span.
🔧 Proposed fix to add actual async work
- const switchTab = async (newTab: string) => {
+ const switchTab = (newTab: string) => {
setLoading(true)
- setTab(newTab)
- setLoading(false) // Flash of loading state
+ // Simulate async work or expensive render
+ setTimeout(() => {
+ setTab(newTab)
+ setLoading(false)
+ }, 0)
}Alternatively, if demonstrating a different issue (e.g., flash of loading UI):
- const switchTab = async (newTab: string) => {
- setLoading(true)
+ const switchTab = (newTab: string) => {
setTab(newTab)
- setLoading(false) // Flash of loading state
}
return (
<div>
- {loading && <Spinner />}
+ <Spinner /> {/* Always shows, causing UI jump */}
<TabContent tab={tab} />
</div>
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const switchTab = async (newTab: string) => { | |
| setLoading(true) | |
| setTab(newTab) | |
| setLoading(false) // Flash of loading state | |
| } | |
| const switchTab = (newTab: string) => { | |
| setLoading(true) | |
| // Simulate async work or expensive render | |
| setTimeout(() => { | |
| setTab(newTab) | |
| setLoading(false) | |
| }, 0) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/review-react/rules/rendering-usetransition-loading.md around
lines 21 - 25, The example's switchTab function is incorrectly marked async but
contains no awaited work, so setLoading(false) runs immediately and doesn't
model a real async loading state; update the switchTab implementation
(referencing switchTab, setLoading, setTab) to either remove the async keyword
if no async work is intended, or include an actual awaited operation (e.g.,
awaiting a fetch or a short delay promise) between setLoading(true) and
setLoading(false) so the loading state spans the async work and correctly
demonstrates the issue.
| If a state value is only consumed in event handlers (not in JSX), subscribing to it via `useState` causes unnecessary re-renders. Defer the read using a ref or by restructuring the component. | ||
|
|
||
| **Incorrect (subscribing to state only used in callback):** | ||
|
|
||
| ```tsx | ||
| function Form() { | ||
| const [draft, setDraft] = useState('') | ||
| const [items, setItems] = useState<string[]>([]) | ||
|
|
||
| // `draft` is read only in this handler, but every keystroke re-renders | ||
| const handleAdd = () => { | ||
| setItems(prev => [...prev, draft]) | ||
| setDraft('') | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <input value={draft} onChange={e => setDraft(e.target.value)} /> | ||
| <button onClick={handleAdd}>Add</button> | ||
| <ItemList items={items} /> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix contradictory “incorrect” example (it currently uses draft in JSX).
The rule says the state is only used in callbacks, but the example binds draft to value in JSX (Line 29), which contradicts the premise (Line 12 and Line 21). Please update the “incorrect” snippet so draft is truly callback-only, or reword the rule text/comment to match the current example.
Suggested correction
- // `draft` is read only in this handler, but every keystroke re-renders
+ // `draft` is also bound to JSX, so this is a controlled input patternOr make the snippet actually callback-only by removing value={draft} and adjusting the explanation accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/review-react/rules/rerender-defer-reads.md around lines 12 -
34, The "incorrect" example contradicts its premise because the Form component
binds draft in JSX; update the snippet so draft is truly only read in callbacks
by removing the value={draft} binding on the input (leaving uncontrolled input
or using a ref) and adjust the explanation to state that draft is only accessed
inside handleAdd (and possibly via a ref), or alternatively reword the rule text
to reflect that draft is currently used in JSX; target symbols: Form, draft,
handleAdd, ItemList.
1 participant
Summary
Add a cross-agent React code review skill under
.agents/skills/review-react/following the Agent Skills standard. The skill includes 25 rules across 4 categories:A
.claude/skills/review-reactsymlink is added so Claude Code also discovers the skill. AGENTS.md references the skill directory for other coding agents.References
Test plan
/menu or auto-invocation.agents/skills/review-react/SKILL.md🤖 Generated with Claude Code