Repository navigation
chore(skills): add project React performance guidance #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Asaf-prog
wants to merge
4
commits into
main
Choose a base branch
from
codex/add-react-performance-skill
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+4,335
−0
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
564afbd
chore(skills): add project React performance guidance
Asaf-prog c3e4c23
fix(skills): correct unsafe React rule examples from review
Asaf-prog 85abf45
fix(skills): complete React rule corrections from re-review
Asaf-prog dbfb99f
fix(skills): resolve remaining React rule review findings
Asaf-prog File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
56 changes: 56 additions & 0 deletions
56
.ai/references/vercel-react-best-practices/rules/advanced-effect-event-deps.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| --- | ||
| title: Do Not Put Effect Events in Dependency Arrays | ||
| impact: LOW | ||
| impactDescription: avoids unnecessary effect re-runs and lint errors | ||
| tags: advanced, hooks, useEffectEvent, dependencies, effects | ||
| --- | ||
|
|
||
| ## Do Not Put Effect Events in Dependency Arrays | ||
|
|
||
| Effect Event functions do not have a stable identity. Their identity intentionally changes on every render. Do not include the function returned by `useEffectEvent` in a `useEffect` dependency array. Keep the actual reactive values as dependencies and call the Effect Event from inside the effect body or subscriptions created by that effect. | ||
|
|
||
| **Incorrect (Effect Event added as a dependency):** | ||
|
|
||
| ```tsx | ||
| import { useEffect, useEffectEvent } from 'react' | ||
|
|
||
| function ChatRoom({ roomId, onConnected }: { | ||
| roomId: string | ||
| onConnected: () => void | ||
| }) { | ||
| const handleConnected = useEffectEvent(onConnected) | ||
|
|
||
| useEffect(() => { | ||
| const connection = createConnection(roomId) | ||
| connection.on('connected', handleConnected) | ||
| connection.connect() | ||
|
|
||
| return () => connection.disconnect() | ||
| }, [roomId, handleConnected]) | ||
| } | ||
| ``` | ||
|
|
||
| Including the Effect Event in dependencies makes the effect re-run every render and triggers the React Hooks lint rule. | ||
|
|
||
| **Correct (depend on reactive values, not the Effect Event):** | ||
|
|
||
| ```tsx | ||
| import { useEffect, useEffectEvent } from 'react' | ||
|
|
||
| function ChatRoom({ roomId, onConnected }: { | ||
| roomId: string | ||
| onConnected: () => void | ||
| }) { | ||
| const handleConnected = useEffectEvent(onConnected) | ||
|
|
||
| useEffect(() => { | ||
| const connection = createConnection(roomId) | ||
| connection.on('connected', handleConnected) | ||
| connection.connect() | ||
|
|
||
| return () => connection.disconnect() | ||
| }, [roomId]) | ||
| } | ||
| ``` | ||
|
|
||
| Reference: [React useEffectEvent: Effect Event in deps](https://react.dev/reference/react/useEffectEvent#effect-event-in-deps) |
55 changes: 55 additions & 0 deletions
55
.ai/references/vercel-react-best-practices/rules/advanced-event-handler-refs.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. Its returned function is not stable: React returns a new one on every render, so keep it out of dependency arrays (see `advanced-effect-event-deps.md`). The subscription still works because the function registered by the effect, from whichever render, always runs the latest handler with the latest props and state. |
42 changes: 42 additions & 0 deletions
42
.ai/references/vercel-react-best-practices/rules/advanced-init-once.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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) |
41 changes: 41 additions & 0 deletions
41
.ai/references/vercel-react-best-practices/rules/advanced-use-latest.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| --- | ||
| title: useEffectEvent for Latest-Value Callbacks | ||
| impact: LOW | ||
| impactDescription: prevents effect re-runs | ||
| tags: advanced, hooks, useEffectEvent, refs, optimization | ||
| --- | ||
|
|
||
| ## useEffectEvent for Latest-Value Callbacks | ||
|
|
||
| Access latest values in callbacks without adding them to dependency arrays. Prevents effect re-runs while avoiding stale closures. | ||
|
|
||
| The function returned by `useEffectEvent` is not stable: React intentionally returns a new one on every render. Call it only from inside effects, and keep it out of dependency arrays (see `advanced-effect-event-deps.md`). It avoids re-runs because it always reads the latest props and state, not because its identity is stable. | ||
|
|
||
| **Incorrect (effect re-runs on every callback change):** | ||
|
|
||
| ```tsx | ||
| function SearchInput({ onSearch }: { onSearch: (q: string) => void }) { | ||
| const [query, setQuery] = useState('') | ||
|
|
||
| useEffect(() => { | ||
| const timeout = setTimeout(() => onSearch(query), 300) | ||
| return () => clearTimeout(timeout) | ||
| }, [query, onSearch]) | ||
| } | ||
| ``` | ||
|
|
||
| **Correct (using React's useEffectEvent):** | ||
|
|
||
| ```tsx | ||
| import { useEffectEvent } from 'react'; | ||
|
|
||
| function SearchInput({ onSearch }: { onSearch: (q: string) => void }) { | ||
| const [query, setQuery] = useState('') | ||
| const onSearchEvent = useEffectEvent(onSearch) | ||
|
|
||
| useEffect(() => { | ||
| const timeout = setTimeout(() => onSearchEvent(query), 300) | ||
| return () => clearTimeout(timeout) | ||
| }, [query]) | ||
| } | ||
| ``` |
44 changes: 44 additions & 0 deletions
44
.ai/references/vercel-react-best-practices/rules/async-api-routes.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| --- | ||
| title: Prevent Waterfall Chains in API Routes | ||
| impact: CRITICAL | ||
| impactDescription: 2-10× improvement | ||
| tags: api-routes, server-actions, waterfalls, parallelization | ||
| --- | ||
|
|
||
| ## Prevent Waterfall Chains in API Routes | ||
|
|
||
| In API routes and Server Actions, start independent operations immediately, even if you don't await them yet. | ||
|
|
||
| **Incorrect (config waits for auth, data waits for both):** | ||
|
|
||
| ```typescript | ||
| export async function GET(request: Request) { | ||
| const session = await auth() | ||
| const config = await fetchConfig() | ||
| const data = await fetchData(session.user.id) | ||
| return Response.json({ data, config }) | ||
| } | ||
| ``` | ||
|
|
||
| **Correct (auth and config start immediately):** | ||
|
|
||
| ```typescript | ||
| export async function GET(request: Request) { | ||
| const sessionPromise = auth() | ||
| const configPromise = fetchConfig() | ||
| // Mark the early start as handled: if auth fails or returns early, a later | ||
| // config rejection must not become an unhandled rejection. | ||
| configPromise.catch(() => {}) | ||
| const session = await sessionPromise | ||
| if (!session) return new Response('Unauthorized', { status: 401 }) | ||
| const [config, data] = await Promise.all([ | ||
| configPromise, | ||
| fetchData(session.user.id) | ||
| ]) | ||
| return Response.json({ data, config }) | ||
| } | ||
| ``` | ||
|
|
||
| Start work before authentication only when it is safe for an unauthenticated caller to trigger: cheap, free of side effects, and independent of the caller. Otherwise wait for authentication first. | ||
|
|
||
| For operations with more complex dependency chains, use `better-all` to automatically maximize parallelism (see Dependency-Based Parallelization). | ||
37 changes: 37 additions & 0 deletions
37
...erences/vercel-react-best-practices/rules/async-cheap-condition-before-await.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| title: Check Cheap Conditions Before Async Flags | ||
| impact: HIGH | ||
| impactDescription: avoids unnecessary async work when a synchronous guard already fails | ||
| tags: async, await, feature-flags, short-circuit, conditional | ||
| --- | ||
|
|
||
| ## Check Cheap Conditions Before Async Flags | ||
|
|
||
| When a branch uses `await` for a flag or remote value and also requires a **cheap synchronous** condition (local props, request metadata, already-loaded state), evaluate the cheap condition **first**. Otherwise you pay for the async call even when the compound condition can never be true. | ||
|
|
||
| This is a specialization of [Defer Await Until Needed](./async-defer-await.md) for `flag && cheapCondition` style checks. | ||
|
|
||
| **Incorrect:** | ||
|
|
||
| ```typescript | ||
| const someFlag = await getFlag() | ||
|
|
||
| if (someFlag && someCondition) { | ||
| // ... | ||
| } | ||
| ``` | ||
|
|
||
| **Correct:** | ||
|
|
||
| ```typescript | ||
| if (someCondition) { | ||
| const someFlag = await getFlag() | ||
| if (someFlag) { | ||
| // ... | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| This matters when `getFlag` hits the network, a feature-flag service, or `React.cache` / DB work: skipping it when `someCondition` is false removes that cost on the cold path. | ||
|
|
||
| Keep the original order if `someCondition` is expensive, depends on the flag, or you must run side effects in a fixed order. |
82 changes: 82 additions & 0 deletions
82
.ai/references/vercel-react-best-practices/rules/async-defer-await.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| --- | ||
| title: Defer Await Until Needed | ||
| impact: HIGH | ||
| impactDescription: avoids blocking unused code paths | ||
| tags: async, await, conditional, optimization | ||
| --- | ||
|
|
||
| ## Defer Await Until Needed | ||
|
|
||
| Move `await` operations into the branches where they're actually used to avoid blocking code paths that don't need them. | ||
|
|
||
| **Incorrect (blocks both branches):** | ||
|
|
||
| ```typescript | ||
| async function handleRequest(userId: string, skipProcessing: boolean) { | ||
| const userData = await fetchUserData(userId) | ||
|
|
||
| if (skipProcessing) { | ||
| // Returns immediately but still waited for userData | ||
| return { skipped: true } | ||
| } | ||
|
|
||
| // Only this branch uses userData | ||
| return processUserData(userData) | ||
| } | ||
| ``` | ||
|
|
||
| **Correct (only blocks when needed):** | ||
|
|
||
| ```typescript | ||
| async function handleRequest(userId: string, skipProcessing: boolean) { | ||
| if (skipProcessing) { | ||
| // Returns immediately without waiting | ||
| return { skipped: true } | ||
| } | ||
|
|
||
| // Fetch only when needed | ||
| const userData = await fetchUserData(userId) | ||
| return processUserData(userData) | ||
| } | ||
| ``` | ||
|
|
||
| **Another example (early return optimization):** | ||
|
|
||
| ```typescript | ||
| // Incorrect: always fetches permissions | ||
| async function updateResource(resourceId: string, userId: string) { | ||
| const permissions = await fetchPermissions(userId) | ||
| const resource = await getResource(resourceId) | ||
|
|
||
| if (!resource) { | ||
| return { error: 'Not found' } | ||
| } | ||
|
|
||
| if (!permissions.canEdit) { | ||
| return { error: 'Forbidden' } | ||
| } | ||
|
|
||
| return await updateResourceData(resource, permissions) | ||
| } | ||
|
|
||
| // Correct: fetches only when needed | ||
| async function updateResource(resourceId: string, userId: string) { | ||
| const resource = await getResource(resourceId) | ||
|
|
||
| if (!resource) { | ||
| return { error: 'Not found' } | ||
| } | ||
|
|
||
| const permissions = await fetchPermissions(userId) | ||
|
|
||
| if (!permissions.canEdit) { | ||
| return { error: 'Forbidden' } | ||
| } | ||
|
|
||
| return await updateResourceData(resource, permissions) | ||
| } | ||
| ``` | ||
|
|
||
| This optimization is especially valuable when the skipped branch is frequently taken, or when the deferred operation is expensive. | ||
|
|
||
| For `await getFlag()` combined with a cheap synchronous guard (`flag && someCondition`), see [Check Cheap Conditions Before Async Flags](./async-cheap-condition-before-await.md). |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.