chore: add vercel-react-best-practices agent skill#25
Merged
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds a comprehensive Vercel React Best Practices skill guide—a structured repository of 60+ performance optimization rules for React and Next.js, organized across nine developer checkpoints: infrastructure, advanced patterns, async parallelization, bundle optimization, client performance, JavaScript algorithms, rendering, re-renders, and server patterns. ChangesVercel React Best Practices Skill Guide
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR installs the
Confidence Score: 5/5Purely additive agent-tooling documentation; no application code is modified. All 76 changed files are documentation and rule markdown under No files require special attention; the stale rule count in Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent / LLM receives React task] --> B{Consult SKILL.md}
B --> C[Identify relevant category]
C --> D1[async-* Waterfall rules]
C --> D2[bundle-* Bundle rules]
C --> D3[server-* Server rules]
C --> D4[client-* Client rules]
C --> D5[rerender-* Re-render rules]
C --> D6[rendering-* Rendering rules]
C --> D7[js-* JS perf rules]
C --> D8[advanced-* Advanced patterns]
D1 & D2 & D3 & D4 & D5 & D6 & D7 & D8 --> E[Read individual rule/*.md]
E --> F[Apply fix / generate code]
B --> G[Or read AGENTS.md for full compiled guide]
G --> F
Reviews (3): Last reviewed commit: "docs(skills): fix bundle-defer-third-par..." | Re-trigger Greptile |
|
CodeAnt AI finished reviewing your PR. |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
.agents/skills/vercel-react-best-practices/SKILL.md-136-139 (1)
136-139:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language identifier to the fenced code block.
This block is missing a language tag, which trips markdown lint and weakens readability.
Proposed fix
-``` +```text rules/async-parallel.md rules/bundle-barrel-imports.md</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/vercel-react-best-practices/SKILL.md around lines 136 - 139,
The fenced code block containing the two rule filenames (the block showing
"rules/async-parallel.md" and "rules/bundle-barrel-imports.md") is missing a
language identifier; update that triple-backtick fence in SKILL.md to include a
language tag (e.g., add "text" immediately after the openingin the code block) so the block becomestext ... ``` to satisfy markdown lint and improve
readability.</details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/rerender-defer-reads.md-27-34 (1)</summary><blockquote> `27-34`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Mark this example as client-only.** Because it reads `window.location.search`, add a short note that this pattern is browser-only (not server-render/runtime-safe). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/rerender-defer-reads.md around lines 27 - 34, The example using ShareButton/handleShare reads window.location.search and must be marked as client-only; update the documentation block around the "Correct (reads on demand, no subscription):" example to add a short note that this pattern is browser-only (not server/runtime-safe) and indicate the component (ShareButton) should be client-only so readers know it cannot run on the server. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md-58-58 (1)</summary><blockquote> `58-58`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove the “reduces memory leaks” claim from benefits.** Using functional `setState` reduces stale-closure risk and callback churn, but it does not directly mitigate memory leaks. Please reword this benefit to avoid overclaiming. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md at line 58, Edit the bullet that starts with "3. **Fewer dependencies**" to remove the overclaim "reduces memory leaks" and replace it with an accurate benefit—e.g., "Simplifies dependency arrays and reduces stale-closure risks / callback churn"—so the line reads something like: "3. **Fewer dependencies** - Simplifies dependency arrays and reduces stale-closure risk and callback churn." Ensure you update only that phrase and keep the original heading and numbering. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md-82-82 (1)</summary><blockquote> `82-82`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Avoid hardcoding engine-specific spread limits in guidance.** The exact max-length numbers are likely to drift across engine versions and can age quickly. Prefer a stable warning without precise thresholds. <details> <summary>Suggested edit</summary> ```diff -This works for small arrays, but can be slower or just throw an error for very large arrays due to spread operator limitations. Maximal array length is approximately 124000 in Chrome 143 and 638000 in Safari 18; exact numbers may vary - see [the fiddle](https://jsfiddle.net/qw1jabsx/4/). Use the loop approach for reliability. +This works for small arrays, but for large arrays spread arguments can hit engine limits or degrade performance. Limits vary by runtime/version; prefer the loop approach for reliability. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md at line 82, Remove the hardcoded numeric engine limits from the guidance in the js-min-max-loop rule and replace them with a stable, version-agnostic warning: keep the note that the spread operator can fail or be slower for very large arrays and recommend using the loop approach for reliability, but do not list exact max-length numbers; update the sentence that currently contains “Maximal array length is approximately 124000 in Chrome 143 and 638000 in Safari 18” to a generic statement about engine-specific limits varying across versions. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/server-parallel-fetching.md-67-67 (1)</summary><blockquote> `67-67`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Import `ReactNode` in the TSX example snippet** The snippet at line 67 uses `ReactNode` without showing an import; in standard modern React + TypeScript setups, `ReactNode` isn’t implicitly available in TSX and should be brought in explicitly (e.g., `import type { ReactNode } from "react"`) or referenced as `React.ReactNode` after importing the React namespace. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/server-parallel-fetching.md at line 67, The TSX snippet uses the type ReactNode in the Layout component signature but doesn't import it; update the example to import the type (or reference React.ReactNode) so the compiler knows the symbol: add an import like import type { ReactNode } from "react" (or change the prop type to React.ReactNode) and keep the function Layout({ children }: { children: ReactNode }) signature intact. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/server-serialization.md-10-10 (1)</summary><blockquote> `10-10`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix overly absolute RSC serialization wording (Line 10)** Line 10 overstates the mechanism: React/Next server→client data uses the RSC Payload transport (not “all object properties into strings”), which is embedded in the initial HTML on first load but fetched as a separate payload on later navigations. Reword to a transport-agnostic claim focused on payload size (e.g., “Crossing the server/client boundary increases the serialized RSC payload size; only pass fields the client actually uses.”) <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/server-serialization.md at line 10, Update the sentence starting "The React Server/Client boundary serializes all object properties into strings..." to a transport-agnostic, accurate statement about payload size—e.g., replace it with "Crossing the server/client boundary increases the serialized RSC payload size; only pass fields the client actually uses." Ensure the new wording removes the incorrect claim about serializing all properties into strings and mentions that payload size matters for page weight/load time. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md-35-88 (1)</summary><blockquote> `35-88`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add `react` imports for `Suspense` and `use` in the TSX snippets** The examples in this doc reference `Suspense` (JSX) and `use(dataPromise)` without importing them; include `import { Suspense, use } from 'react'` in the relevant code blocks (around lines 35-88) for copy/paste safety. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md around lines 35 - 88, The TSX examples reference Suspense and the use hook but don't import them; update the snippets (the Page, DataDisplay, and DataSummary examples) to include an import statement importing Suspense and use from React (e.g., import { Suspense, use } from 'react') at the top of each code block so the examples are copy/paste-safe and the symbols Suspense and use resolve correctly. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md-19-25 (1)</summary><blockquote> `19-25`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Define `loading` in the examples to keep snippets runnable.** Line 22 and Line 38 reference `loading`, but it is never declared in either `Container` example. Please make the sample self-contained. <details> <summary>Suggested doc fix</summary> ```diff -function Container() { +function Container({ loading }: { loading: boolean }) { return ( <div> {loading && <LoadingSkeleton />} </div> ) } ``` ```diff -function Container() { +function Container({ loading }: { loading: boolean }) { return ( <div> {loading && loadingSkeleton} </div> ) } ``` </details> Also applies to: 35-41 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md around lines 19 - 25, The examples reference a variable named loading that is never declared, making the snippets unrunnable; update each Container example (the function named Container that returns JSX with {loading && <LoadingSkeleton />}) to declare loading locally (e.g., add a simple const loading = true/false or useState: const [loading, setLoading] = useState(true)) so the snippet is self-contained and runnable, and ensure any necessary React imports are present if using useState. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md (1)</summary><blockquote> `57-65`: _⚡ Quick win_ **Make the `useSWRSubscription` callback signature explicit for API safety.** This snippet may be copied into strict TS projects; using the explicit SWR subscription callback parameters avoids version/signature ambiguity and reduces integration errors. <details> <summary>Suggested snippet adjustment</summary> ```diff - useSWRSubscription('global-keydown', () => { + useSWRSubscription('global-keydown', (_key, { next }) => { const handler = (e: KeyboardEvent) => { if (e.metaKey && keyCallbacks.has(e.key)) { - keyCallbacks.get(e.key)!.forEach(cb => cb()) + keyCallbacks.get(e.key)!.forEach(cb => cb()) + next(null, e) } } window.addEventListener('keydown', handler) return () => window.removeEventListener('keydown', handler) }) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md around lines 57 - 65, The SWR subscription callback should declare the expected parameters to avoid signature ambiguity: update the useSWRSubscription second-argument from an anonymous parameterless function to an explicit callback signature (e.g., (key, subscribeCallback) or (key: string, onData: (value: any) => void) depending on your SWR types) and keep your handler logic inside it; ensure you still add/remove the window 'keydown' listener with the same handler and reference the existing keyCallbacks map and handler function name to avoid capturing wrong scope. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-cache-property-access.md (1)</summary><blockquote> `10-10`: _⚡ Quick win_ **Scope this rule to measured hot paths instead of general usage.** As written, it reads as universal advice; caching property access is often negligible on modern engines and can reduce readability. Suggest adding “apply only after profiling on hot loops.” Also applies to: 12-27 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-cache-property-access.md at line 10, Update the rule "Cache object property lookups in hot paths." to scope guidance to measured hot paths only: change the universal phrasing to say developers should only cache property access after profiling confirms a hotspot (e.g., "apply only after profiling on hot loops"), and add a short rationale that modern engines often optimize access and caching can hurt readability; apply the same wording change to the duplicated guidance in lines covering the other instances (the block referenced by the comment for lines 12-27) so all occurrences consistently require profiling before applying this optimization. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md (1)</summary><blockquote> `10-10`: _⚡ Quick win_ **Add guardrails for unbounded module-level cache growth.** This guidance should explicitly note eviction/size limits (or request-scoped caching) for high-cardinality inputs; otherwise examples promote memory growth in long-lived processes. Also applies to: 33-41, 78-78 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md at line 10, The examples that recommend using a module-level Map to cache function results lack guardrails against unbounded growth—update the guidance to recommend adding explicit eviction/size limits (e.g., LRU cap), TTLs, or using request-scoped/in-memory caches for high-cardinality inputs, and show a short pattern for limiting entries (or switching to a bounded cache library) wherever the module-level Map cache is used in the examples so readers know how to prevent memory leaks. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In @.agents/skills/vercel-react-best-practices/metadata.json:
- Line 5: The "abstract" field in the metadata.json currently states "40+ rules"
which conflicts with SKILL.md and the PR describing 70 rules; update the
metadata.json abstract to match the canonical count (e.g., "70 rules" or "70+
rules") or otherwise sync both sources so they match, ensuring the "abstract"
string in metadata.json is changed to the agreed-upon rule count and any other
summary fields that reference rule count are updated accordingly.In @.agents/skills/vercel-react-best-practices/README.md:
- Around line 58-86: The markdown code fence that begins with "
markdown" (the front-matter block under the "Rule Title Here" section) is not closed, causing the rest of the README to render as code; close that fenced block by adding a terminating "" immediately after the optional explanatory text or the
"Reference: Link" line so the subsequent "## File Naming Convention"
header renders as normal text. Ensure you update the content around the "Rule
Title Here" examples (the incorrect/correct code examples and any following
explanatory text) to be inside the fenced block before the closing backticks.In
@.agents/skills/vercel-react-best-practices/rules/advanced-effect-event-deps.md:
- Line 10: Update the sentence in advanced-event-handler-refs.md that currently
claims useEffectEvent "creates a stable function reference" to match React docs:
change it to state that functions returned by useEffectEvent do not have a
stable identity and should be omitted from useEffect dependency arrays, and
advise keeping reactive values as dependencies and calling the Effect Event
inside the effect body; reference useEffectEvent and the same omit-from-deps
guidance as in advanced-effect-event-deps.md.In @.agents/skills/vercel-react-best-practices/rules/advanced-init-once.md:
- Around line 10-11: Update the guidance to state that the "module-level guard"
pattern (e.g., a didInit module-variable) is only appropriate in browser/client
entry modules and only for non-request/user-specific, idempotent initialization;
continue to recommend avoiding useEffect([]) for app-wide init but clarify that
didInit must not be used in Next.js server/SSR or other request-scoped contexts
because module variables are process-wide and can leak state across requests—add
a clear warning against mutable module state on the server and suggest
server-safe alternatives (per-request initialization or top-level init in the
server entry/module designed for request scoping) and reference the terms
didInit, useEffect([]), module-level guard and client entry points so
maintainers can locate the rule text to update.In @.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md:
- Line 31: Update the rule text in bundle-conditional.md to soften the claim
thattypeof window !== 'undefined'will prevent SSR bundling or always improve
server bundle size/build speed; rephrase to say the check prevents server-side
execution of the guarded branch but does not universally guarantee the module or
chunk is excluded from SSR tracing, and add a recommendation to validate effects
by inspecting your build output (e.g., emitted chunks or bundle stats) rather
than assuming automatic exclusion.In
@.agents/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md:
- Around line 31-49: The example uses next/dynamic(..., { ssr: false }) (symbol:
Analytics) directly inside the Server Component RootLayout, which is invalid
because { ssr: false } requires a Client Component; fix by moving the dynamic
import into a small Client Component (create AnalyticsClient with 'use client'
that imports dynamic and defines Analytics via dynamic(() =>
import('@vercel/analytics/react').then(m => m.Analytics), { ssr: false }) and
returns ), then import and render AnalyticsClient from the Server
RootLayout instead of rendering Analytics directly.In @.agents/skills/vercel-react-best-practices/rules/bundle-preload.md:
- Line 50: Update the sentence in bundle-preload.md (and the similar sentence in
bundle-conditional.md) that currently claims thetypeof window !== 'undefined'
check “prevents bundling … for SSR” and “optimizing server bundle size and build
speed”: change it to say the guard prevents the preload/import logic from
running at server runtime and can help avoid executing client-only code during
SSR, but does not by itself guarantee that bundlers or module tracing will
exclude those modules from the server bundle; keep the example checktypeof window !== 'undefined'and replace the absolutist phrasing with this more
precise wording.In @.agents/skills/vercel-react-best-practices/rules/client-swr-dedup.md:
- Line 38: The import in the doc uses non-standard hook names; replace the local
alias import "useImmutableSWR" with the official named export "useSWRImmutable"
from "swr/immutable" and ensure the mutation hook is imported as the default
export "useSWRMutation" from "swr/mutation" (i.e., use import useSWRMutation
from 'swr/mutation'); update any examples that reference useImmutableSWR to
useSWRImmutable and adjust mutation examples to use the default-imported
useSWRMutation to match the official hook exports.In @.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md:
- Around line 49-50: Fix two issues: parse cookies by splitting only on the
first '=' instead of using c.split('=') (use c.indexOf('=') and slice to get
name and value) so cookie values containing '=' are preserved when building the
cookie map from document.cookie, and update the storage event handling so that
when event.key === null (e.g., localStorage.clear()) you invalidate the entire
cache/store rather than leaving stale entries — locate the cookie parsing code
around document.cookie and the storage event listener that checks event.key and
add a branch to clear the full cache (or call the existing invalidateAll/clear
method) when key is null.In
@.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md:
- Around line 86-90: The code uses window.requestIdleCallback at
module-evaluation time which will break SSR; update the scheduleIdleWork
initializer to be SSR-safe by checking for runtime availability (e.g. typeof
window !== "undefined" and typeof window.requestIdleCallback === "function") or
use globalThis similarly, and fall back to setTimeout only when
requestIdleCallback isn't available or window/globalThis is undefined; update
the reference to scheduleIdleWork so it is initialized lazily/conditionally
(using the symbol scheduleIdleWork and the feature check for
window.requestIdleCallback) to avoid accessing window during server-side module
initialization.In
@.agents/skills/vercel-react-best-practices/rules/rendering-script-defer-async.md:
- Around line 53-63: Update the example in the Page component to stop using
strategy="beforeInteractive" as a generic defer replacement: change the second
Script instance (the Script importing "/scripts/utils.js" inside the exported
function Page) to use strategy="afterInteractive" instead, so both Script usages
use strategy="afterInteractive" while keeping the same import of Script from
'next/script'.In
@.agents/skills/vercel-react-best-practices/rules/rendering-usetransition-loading.md:
- Around line 10-74: The rule incorrectly presents useTransition as a universal
replacement for request lifecycle loading and shows startTransition wrapping an
async/await which is misleading; update the text and example to state that
useTransition is for marking non-urgent UI updates (symbols: useTransition,
startTransition, isPending) and that React only treats the synchronous updates
inside startTransition as part of the transition, so move network work outside
the transition (e.g., await fetchResults(...) then call startTransition(() =>
setResults(data))) or clearly explain the caveat in the handleSearch example
(handleSearch, fetchResults, setResults), and remove/soften claims about “error
resilience” and automatic cancellation of in-flight async work.In
@.agents/skills/vercel-react-best-practices/rules/rerender-memo-with-default-value.md:
- Around line 12-14: Reword the rule to clarify that creating a default
parameter inside a component wrapped with memo() does not itself break memo’s
shallow prop comparison; instead explain that the real issue is unstable default
value instances (arrays/objects/functions) being created on each render and then
passed to child components, causing unnecessary re-renders of those descendants.
Update the explanatory text and all examples that reference memo() and default
parameters (the current paragraphs around the existing lines 12–14 and the
example block around 16–38) to show: 1) a corrected explanation mentioning memo
compares incoming props before render, 2) an example where the unstable default
is passed to a child and how extracting it to a module-level constant or using
useMemo/useCallback stabilizes it, and 3) adjust any guidance strings to
recommend stabilizing values passed to descendants rather than claiming memo
itself is broken.In @.agents/skills/vercel-react-best-practices/rules/server-cache-lru.md:
- Around line 23-28: The snippet uses a truthy check on cache.get(id) which
drops legitimate falsy cached values (e.g., null for "not found"); replace the
truthy conditional with an existence check using cache.has(id) and only call
cache.get(id) after confirming presence, e.g., use cache.has(id) to decide
whether to return the cached value and otherwise run db.user.findUnique({ where:
{ id } }), then cache.set(id, user) and return user so null results are
preserved in the cache.
Minor comments:
In
@.agents/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md:
- Around line 35-88: The TSX examples reference Suspense and the use hook but
don't import them; update the snippets (the Page, DataDisplay, and DataSummary
examples) to include an import statement importing Suspense and use from React
(e.g., import { Suspense, use } from 'react') at the top of each code block so
the examples are copy/paste-safe and the symbols Suspense and use resolve
correctly.In @.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md:
- Line 82: Remove the hardcoded numeric engine limits from the guidance in the
js-min-max-loop rule and replace them with a stable, version-agnostic warning:
keep the note that the spread operator can fail or be slower for very large
arrays and recommend using the loop approach for reliability, but do not list
exact max-length numbers; update the sentence that currently contains “Maximal
array length is approximately 124000 in Chrome 143 and 638000 in Safari 18” to a
generic statement about engine-specific limits varying across versions.In @.agents/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md:
- Around line 19-25: The examples reference a variable named loading that is
never declared, making the snippets unrunnable; update each Container example
(the function named Container that returns JSX with {loading && }) to declare loading locally (e.g., add a simple const loading = true/false
or useState: const [loading, setLoading] = useState(true)) so the snippet is
self-contained and runnable, and ensure any necessary React imports are present
if using useState.In @.agents/skills/vercel-react-best-practices/rules/rerender-defer-reads.md:
- Around line 27-34: The example using ShareButton/handleShare reads
window.location.search and must be marked as client-only; update the
documentation block around the "Correct (reads on demand, no subscription):"
example to add a short note that this pattern is browser-only (not
server/runtime-safe) and indicate the component (ShareButton) should be
client-only so readers know it cannot run on the server.In
@.agents/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md:
- Line 58: Edit the bullet that starts with "3. Fewer dependencies" to
remove the overclaim "reduces memory leaks" and replace it with an accurate
benefit—e.g., "Simplifies dependency arrays and reduces stale-closure risks /
callback churn"—so the line reads something like: "3. Fewer dependencies -
Simplifies dependency arrays and reduces stale-closure risk and callback churn."
Ensure you update only that phrase and keep the original heading and numbering.In
@.agents/skills/vercel-react-best-practices/rules/server-parallel-fetching.md:
- Line 67: The TSX snippet uses the type ReactNode in the Layout component
signature but doesn't import it; update the example to import the type (or
reference React.ReactNode) so the compiler knows the symbol: add an import like
import type { ReactNode } from "react" (or change the prop type to
React.ReactNode) and keep the function Layout({ children }: { children:
ReactNode }) signature intact.In @.agents/skills/vercel-react-best-practices/rules/server-serialization.md:
- Line 10: Update the sentence starting "The React Server/Client boundary
serializes all object properties into strings..." to a transport-agnostic,
accurate statement about payload size—e.g., replace it with "Crossing the
server/client boundary increases the serialized RSC payload size; only pass
fields the client actually uses." Ensure the new wording removes the incorrect
claim about serializing all properties into strings and mentions that payload
size matters for page weight/load time.In @.agents/skills/vercel-react-best-practices/SKILL.md:
- Around line 136-139: The fenced code block containing the two rule filenames
(the block showing "rules/async-parallel.md" and
"rules/bundle-barrel-imports.md") is missing a language identifier; update that
triple-backtick fence in SKILL.md to include a language tag (e.g., add "text"
immediately after the openingin the code block) so the block becomestext ... ``` to satisfy markdown lint and improve readability.
Nitpick comments:
In @.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md:
- Around line 57-65: The SWR subscription callback should declare the expected
parameters to avoid signature ambiguity: update the useSWRSubscription
second-argument from an anonymous parameterless function to an explicit callback
signature (e.g., (key, subscribeCallback) or (key: string, onData: (value: any)
=> void) depending on your SWR types) and keep your handler logic inside it;
ensure you still add/remove the window 'keydown' listener with the same handler
and reference the existing keyCallbacks map and handler function name to avoid
capturing wrong scope.In
@.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md:
- Line 10: The examples that recommend using a module-level Map to cache
function results lack guardrails against unbounded growth—update the guidance to
recommend adding explicit eviction/size limits (e.g., LRU cap), TTLs, or using
request-scoped/in-memory caches for high-cardinality inputs, and show a short
pattern for limiting entries (or switching to a bounded cache library) wherever
the module-level Map cache is used in the examples so readers know how to
prevent memory leaks.In
@.agents/skills/vercel-react-best-practices/rules/js-cache-property-access.md:
- Line 10: Update the rule "Cache object property lookups in hot paths." to
scope guidance to measured hot paths only: change the universal phrasing to say
developers should only cache property access after profiling confirms a hotspot
(e.g., "apply only after profiling on hot loops"), and add a short rationale
that modern engines often optimize access and caching can hurt readability;
apply the same wording change to the duplicated guidance in lines covering the
other instances (the block referenced by the comment for lines 12-27) so all
occurrences consistently require profiling before applying this optimization.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `60457d60-aa13-4d1b-969f-26b161bb43ef` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 3843f8377ea92d18bef4337fda6c50667ced6a70 and d52477ed95c30846263b9c12b5e45a85590d1970. </details> <details> <summary>📒 Files selected for processing (76)</summary> * `.agents/skills/vercel-react-best-practices/AGENTS.md` * `.agents/skills/vercel-react-best-practices/README.md` * `.agents/skills/vercel-react-best-practices/SKILL.md` * `.agents/skills/vercel-react-best-practices/metadata.json` * `.agents/skills/vercel-react-best-practices/rules/_sections.md` * `.agents/skills/vercel-react-best-practices/rules/_template.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-effect-event-deps.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-init-once.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-use-latest.md` * `.agents/skills/vercel-react-best-practices/rules/async-api-routes.md` * `.agents/skills/vercel-react-best-practices/rules/async-cheap-condition-before-await.md` * `.agents/skills/vercel-react-best-practices/rules/async-defer-await.md` * `.agents/skills/vercel-react-best-practices/rules/async-dependencies.md` * `.agents/skills/vercel-react-best-practices/rules/async-parallel.md` * `.agents/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-analyzable-paths.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-dynamic-imports.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-preload.md` * `.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md` * `.agents/skills/vercel-react-best-practices/rules/client-localstorage-schema.md` * `.agents/skills/vercel-react-best-practices/rules/client-passive-event-listeners.md` * `.agents/skills/vercel-react-best-practices/rules/client-swr-dedup.md` * `.agents/skills/vercel-react-best-practices/rules/js-batch-dom-css.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-property-access.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md` * `.agents/skills/vercel-react-best-practices/rules/js-combine-iterations.md` * `.agents/skills/vercel-react-best-practices/rules/js-early-exit.md` * `.agents/skills/vercel-react-best-practices/rules/js-flatmap-filter.md` * `.agents/skills/vercel-react-best-practices/rules/js-hoist-regexp.md` * `.agents/skills/vercel-react-best-practices/rules/js-index-maps.md` * `.agents/skills/vercel-react-best-practices/rules/js-length-check-first.md` * `.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md` * `.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md` * `.agents/skills/vercel-react-best-practices/rules/js-set-map-lookups.md` * `.agents/skills/vercel-react-best-practices/rules/js-tosorted-immutable.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-activity.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-conditional-render.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-content-visibility.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hydration-suppress-warning.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-resource-hints.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-script-defer-async.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-svg-precision.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-usetransition-loading.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-defer-reads.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-dependencies.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-derived-state-no-effect.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-derived-state.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-lazy-state-init.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-memo-with-default-value.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-memo.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-move-effect-to-event.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-no-inline-components.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-simple-expression-in-memo.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-split-combined-hooks.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-transitions.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-use-deferred-value.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-use-ref-transient-values.md` * `.agents/skills/vercel-react-best-practices/rules/server-after-nonblocking.md` * `.agents/skills/vercel-react-best-practices/rules/server-auth-actions.md` * `.agents/skills/vercel-react-best-practices/rules/server-cache-lru.md` * `.agents/skills/vercel-react-best-practices/rules/server-cache-react.md` * `.agents/skills/vercel-react-best-practices/rules/server-dedup-props.md` * `.agents/skills/vercel-react-best-practices/rules/server-hoist-static-io.md` * `.agents/skills/vercel-react-best-practices/rules/server-no-shared-module-state.md` * `.agents/skills/vercel-react-best-practices/rules/server-parallel-fetching.md` * `.agents/skills/vercel-react-best-practices/rules/server-parallel-nested-fetching.md` * `.agents/skills/vercel-react-best-practices/rules/server-serialization.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…st-practices - metadata.json: fix stale rule count (40+ -> 70) - README.md: close unterminated markdown fence; note build tooling lives upstream - SKILL.md: add language tag to fenced rule-list block - advanced-event-handler-refs: correct useEffectEvent identity wording - advanced-init-once: scope module-level guard to client entry points - bundle-conditional / bundle-preload: soften absolute SSR bundling claims - bundle-defer-third-party: ssr:false requires a Client Component - client-swr-dedup: use official swr/immutable + default swr/mutation imports - js-cache-storage: split cookie on first '=', clear cache on storage key===null - js-request-idle-callback: SSR-safe scheduleIdleWork - rendering-script-defer-async: afterInteractive instead of beforeInteractive - rendering-usetransition-loading: correct async/transition guidance - rerender-memo-with-default-value: clarify memo wording - server-cache-lru: use cache.has() to preserve falsy cached values
24c3077) The 'Correct' example used next/dynamic(..., { ssr: false }) directly inside RootLayout, but App Router RootLayout is a Server Component and { ssr: false } is not supported there. Wrap the dynamic import in a 'use client' component and render it from the Server RootLayout. Addresses the remaining open CodeRabbit review thread on this file.
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Owner
Author
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Sequence DiagramThis PR adds a React and Next.js best-practices skill that the agent uses to analyze pull requests and surface performance and rendering guidance based on structured rules. sequenceDiagram
participant Developer
participant PRSystem as PR system
participant Agent
participant ReactSkill as React best practices skill
Developer->>PRSystem: Open or update PR with React/Next changes
PRSystem->>Agent: Request PR analysis
Agent->>ReactSkill: Load metadata and rule documents
ReactSkill-->>Agent: Return applicable rules and recommendations
Agent-->>PRSystem: Post review comments with React/Next best-practice guidance
PRSystem-->>Developer: Show performance and best-practice suggestions
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
User description
Adds the
vercel-react-best-practicesagent skill (70 React/Next perf rules across 8 categories) under.agents/skills/vercel-react-best-practices/.npx skills add https://github.com/vercel-labs/agent-skills --skill vercel-react-best-practices(Project scope).SKILL.mdreviewed.No application code touched — additive, agent-tooling only.
Summary by CodeRabbit
CodeAnt-AI Description
Add and correct the Vercel React best-practices skill for agents
What Changed
Impact
✅ Clearer React and Next.js optimization guidance✅ Fewer incorrect performance recommendations✅ Safer examples for agent-generated code💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.