-
Notifications
You must be signed in to change notification settings - Fork 1
feat(config): add react-best-practices agent skill #654
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive React Best Practices skill documentation containing 40+ rules across 8 performance-focused categories, complete with explanations, code examples, and impact metrics for optimization techniques. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @.opencode/skill/react-best-practices/AGENTS.md:
- Around line 1487-1507: Update the guidance for using the Activity component to
state the minimum React version required: mention that the Activity component
(import { Activity } from 'react') is available starting in React 19.2+, so
consumers on React 18 or earlier will get import errors; add a short note or
parenthetical next to the Usage/heading such as "Available in React 19.2+" and
ensure references to the Activity symbol remain unchanged.
- Around line 738-809: Add a clear minimum Next.js version note for the after()
API by updating the AGENTS.md content around the after() examples and the
"Important notes" section to state that after() became stable in Next.js v15.1.0
(include "Next.js v15.1.0+" or similar), so readers know the feature
availability; reference the after() symbol in the text and the Next.js docs link
already present to guide users.
In @.opencode/skill/react-best-practices/rules/advanced-use-latest.md:
- Around line 14-22: The current useLatest updates ref.current in useEffect
which can lag; modify the useLatest function so it sets ref.current = value
synchronously during render (i.e., immediately after creating ref with useRef)
and remove the useEffect (or keep it only if you must for some edge case),
ensuring the returned ref from useLatest always contains the latest value for
callers; reference the useLatest function, the ref variable from useRef, and the
current useEffect invocation when making the change.
In @.opencode/skill/react-best-practices/rules/async-dependencies.md:
- Line 10: The doc recommends the unfindable "better-all" library; fix this by
removing or replacing that recommendation and clarifying install/status: either
delete the "better-all" mention in the rule text, or swap it for a public,
maintained alternative such as "p-all" or "p-queue" and update example usages
(e.g., the `better-all` reference and `await this.$.user` example) to match the
chosen library's API; if "better-all" must remain, add a clear note about its
private status and explicit installation/source instructions and maintenance
caveats.
In @.opencode/skill/react-best-practices/rules/async-suspense-boundaries.md:
- Around line 79-87: Add a short note stating that the example using the use
hook requires React 19+ so it will not work in earlier React versions; update
the documentation near the DataDisplay and DataSummary examples to include a
brief version badge or one-line sentence like “Requires React 19+ (use hook
support)” so readers know the prerequisite for using use with promises.
In @.opencode/skill/react-best-practices/rules/bundle-conditional.md:
- Line 22: The catch handler calls setEnabled(false) but no setter exists in
this component (it only receives enabled as a prop), causing a runtime
ReferenceError; fix by replacing that call with a local state setter (create
e.g. const [loadError, setLoadError] = useState(false) or const [localEnabled,
setLocalEnabled] = useState(enabled) and call setLocalEnabled(false) or
setLoadError(true) in the .catch), or simply remove the .catch handler entirely
so you don't reference undefined setEnabled; ensure you update any logic that
relied on setEnabled to use the new local state (references: setEnabled,
enabled, the promise .catch in this diff).
- Line 31: Update the explanation in bundle-conditional.md for the `typeof
window !== 'undefined'` check: replace the incorrect claim that it "prevents
bundling this module for SSR" with wording that it prevents runtime execution on
the server but does not affect bundler inclusion; add a brief note referencing
build-time exclusions (e.g., framework-specific SSR flags or conditional
compilation) as the correct way to omit code from server bundles and keep the
example focused on execution-only behavior.
In @.opencode/skill/react-best-practices/rules/bundle-defer-third-party.md:
- Around line 31-49: The example uses dynamic(() => import(...).then(m =>
m.Analytics), { ssr: false }) which is unnecessary with `@vercel/analytics`
v1.5.0; replace the dynamic import by importing Analytics directly and render it
in RootLayout (remove dynamic and ssr:false usage referencing Analytics and
RootLayout), or if you truly need lazy-loading move the dynamic import into a
'use client' wrapper component and ensure you mount it only after hydration
(e.g., via useEffect or a mount flag) to avoid hydration mismatches.
In @.opencode/skill/react-best-practices/rules/client-event-listeners.md:
- Around line 68-73: The example in Profile uses inline arrow callbacks passed
into useKeyboardShortcut which create new function references each render and
force the effect to re-run; update the example to use stable, memoized callbacks
(e.g., wrap handlers with React's useCallback or extract named handler
functions) and pass those memoized functions into useKeyboardShortcut so the Map
insertion/removal only runs when the actual handler logic changes; reference the
Profile component and useKeyboardShortcut in the edit.
In @.opencode/skill/react-best-practices/rules/client-swr-dedup.md:
- Around line 37-43: The current example imports a non-existent hook
useImmutableSWR from '@/lib/swr' in the StaticContent example; replace or remove
it: either delete the example block or change the import to a real SWR API
(e.g., import { useSWRImmutable } from 'swr' or use useSWR with appropriate
options) and keep the component name StaticContent and the same data usage so
the snippet compiles against the real library instead of '@/lib/swr'.
In @.opencode/skill/react-best-practices/rules/js-hoist-regexp.md:
- Around line 27-35: The snippet uses escapeRegex(query) but doesn't define it;
add a safe helper named escapeRegex and use it when building the RegExp in
Highlighter (the useMemo that constructs regex) so queries with special regex
characters are escaped; implement escapeRegex to escape characters like . * + ?
^ $ { } ( ) | [ ] \ / and ensure Highlighter continues to call
escapeRegex(query) before new RegExp(...) to avoid broken or unsafe regexes.
In @.opencode/skill/react-best-practices/rules/rendering-content-visibility.md:
- Around line 1-38: Update the compatibility note in the
rendering-content-visibility rule to state that content-visibility is now
broadly supported across major engines (Chrome/Chromium v85+, Firefox v125+,
Safari/WebKit 2024–2025) instead of implying limited support; in the doc
(title/description and the example block referencing .message-item and the
MessageList component) add a short sentence advising graceful fallback for older
browsers (pre-v125 Firefox, older Safari) such as rendering normally when the
property is unsupported and optionally feature-detect via CSS.supports; keep the
CSS example for .message-item and the MessageList example unchanged but append
the compatibility/fallback note so readers know when and how to degrade.
In @.opencode/skill/react-best-practices/rules/server-after-nonblocking.md:
- Around line 45-46: The rule's example uses the async Next.js APIs headers()
and cookies(), which only exist in Next.js 15+; update the rule to explicitly
state the Next.js version requirement by adding a note or frontmatter indicating
this pattern requires Next.js 15 or later (and that Next.js 13–14 use
synchronous headers/cookies APIs), and mention the specific symbols headers()
and cookies() so readers know which APIs are version-dependent.
In @.opencode/skill/react-best-practices/rules/server-cache-react.md:
- Around line 14-24: Mention that React.cache (used in the getCurrentUser
example) is an experimental/canary-only API: update the text around the example
to explicitly state that React.cache is part of React's experimental/canary
channel (or only available when a framework vendors an experimental React), may
be absent in stable releases, and can change between versions; reference the
symbol React.cache and the example function getCurrentUser so readers know the
requirement and risk.
In @.opencode/skill/react-best-practices/SKILL.md:
- Around line 111-115: The fenced code block that lists rule file paths (the
triple-backtick block containing "rules/async-parallel.md",
"rules/bundle-barrel-imports.md", "rules/_sections.md") is missing a language
identifier; update that block to use a language tag (e.g., change ``` to
```text) so markdownlint recognizes it as a plain-text fenced code block.
🧹 Nitpick comments (21)
.opencode/skill/react-best-practices/rules/bundle-preload.md (1)
50-50: Consider clarifying the bundling explanation.The current phrasing "prevents bundling preloaded modules for SSR" may be slightly misleading. The runtime check prevents the dynamic import from executing on the server, not bundling itself. The module would still be included in the client bundle when dynamically imported.
📝 Suggested clarification
-The `typeof window !== 'undefined'` check prevents bundling preloaded modules for SSR, optimizing server bundle size and build speed. +The `typeof window !== 'undefined'` check prevents executing the dynamic import on the server during SSR, avoiding unnecessary server-side module loading and keeping server execution focused on rendering..opencode/skill/react-best-practices/rules/rendering-conditional-render.md (1)
8-10: Consider clarifying which falsy values actually render.The phrase "or other falsy values that render" could be more specific. In React, only
0,NaN, and""(empty string) render from common falsy values, whilefalse,null, andundefinedrender nothing. Consider explicitly listing them for clarity.✏️ Optional clarification
-Use explicit ternary operators (`? :`) instead of `&&` for conditional rendering when the condition can be `0`, `NaN`, or other falsy values that render. +Use explicit ternary operators (`? :`) instead of `&&` for conditional rendering when the condition can be `0`, `NaN`, or `""` (empty string) – falsy values that render in React..opencode/skill/react-best-practices/rules/js-hoist-regexp.md (1)
25-25: Remove or integrate the unused EMAIL_REGEX constant.The
EMAIL_REGEXconstant is defined but never used in theHighlightercomponent example below. This creates confusion about its purpose and relationship to the example.Either remove it or add a separate example that demonstrates its use.
♻️ Suggested fix
Remove the unused constant:
-const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ - function Highlighter({ text, query }: Props) {Or add a brief comment explaining it's a separate example:
+// Example of a static regex at module scope: const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ + +// Example of dynamic regex with useMemo:.opencode/skill/react-best-practices/rules/js-length-check-first.md (3)
14-23: Highlight the mutation bug as a correctness issue.While line 23 correctly identifies the performance problem, it doesn't explicitly call out that
.sort()mutates the original arrays. For AI agent guidance, this correctness issue should be more prominent than the performance concern.📝 Suggested enhancement to the explanation
After line 21, consider adding:
}
+Critical issue: This code mutates both
currentandoriginalarrays in-place, which is a correctness bug that can cause unintended side effects in the calling code.Two O(n log n) sorts run even when
current.lengthis 5 andoriginal.lengthis 100. There is also overhead of joining the arrays and comparing the strings.</details> --- `28-43`: **Update comment to match the actual code.** Line 33's comment mentions "sort/join" but the code only sorts and does element-by-element comparison—it doesn't use `.join()`. <details> <summary>📝 Suggested fix for the comment</summary> ```diff - // Only sort/join when lengths match + // Only sort when lengths match const currentSorted = current.toSorted()
45-49: Consider clarifying the benefits list.Lines 46-47 mention "joining" and "joined strings," but the correct example doesn't use
.join()at all. While technically accurate (the correct version avoids joins compared to the incorrect version), this could be clearer for AI agents.📝 Optional clarification
This new approach is more efficient because: -- It avoids the overhead of sorting and joining the arrays when lengths differ -- It avoids consuming memory for the joined strings (especially important for large arrays) +- It avoids the overhead of sorting when lengths differ (O(1) check vs O(n log n) sort) +- It uses element-by-element comparison instead of string joining, avoiding memory allocation - It avoids mutating the original arrays - It returns early when a difference is found.opencode/skill/react-best-practices/rules/rendering-svg-precision.md (1)
1-10: Clear documentation structure with appropriate impact classification.The frontmatter and introduction are well-structured. The "LOW" impact level is appropriate for precision optimization, and the note about viewBox size dependency is good context.
Optional: Consider adding guidance on when to preserve precision
While the current text mentions that "optimal precision depends on the viewBox size," you might consider adding a brief note about scenarios where higher precision should be maintained (e.g., very small viewBoxes, technical/engineering diagrams, or animations that rely on precise coordinates). This would help developers make informed decisions.
However, this is purely optional as the current version is clear and functional.
.opencode/skill/react-best-practices/rules/js-cache-storage.md (1)
44-53: Consider more robust cookie parsing.The current parsing
c.split('=')without a limit could incorrectly handle cookie values containing=characters. For production use, considerc.split(/=(.+)/)to split only on the first=, or use a more robust cookie parsing library.For documentation purposes, the current example is clear and sufficient.
📝 More robust cookie parsing example
function getCookie(name: string) { if (!cookieCache) { cookieCache = Object.fromEntries( document.cookie.split('; ').map(c => { const [key, ...valueParts] = c.split('=') return [key, valueParts.join('=')] }) ) } return cookieCache[name] }.opencode/skill/react-best-practices/rules/js-cache-function-results.md (1)
1-56: LGTM! Effective function memoization pattern.The Map-based caching correctly avoids redundant computation for repeated function calls with the same inputs. The React component example clearly demonstrates the performance benefit.
For high-traffic production scenarios with many unique inputs, consider implementing cache size limits (LRU eviction) or TTL to prevent unbounded memory growth. For most use cases, the unbounded cache shown here is perfectly acceptable.
.opencode/skill/react-best-practices/rules/rendering-hydration-no-flicker.md (2)
55-78: Verify the "no flicker" claim and consider script placement optimization.The inline script pattern shown here is a common approach and does prevent hydration mismatch. However, the claim of "no flicker" (line 80) may be overstated depending on browser behavior and timing:
Script placement: The script executes after the
divis already in the DOM (the<script>tag comes after the closing</div>). During HTML parsing, the browser may paint the div before executing the script, potentially causing a brief flash.Better practice: For truly flicker-free rendering, place the script in the document
<head>or immediately before the target element. Many frameworks (e.g., next-themes) inject a blocking script in<head>that runs before any body content renders.className overwriting: Line 69 directly assigns
el.className = theme, which replaces all existing classes. If the div needs other CSS classes for layout or styling, they would be lost. Consider usingclassList.add()or documenting that this assumes the element has no other classes.Security note: The documentation doesn't mention that
dangerouslySetInnerHTMLbypasses React's XSS protection. While this use case is relatively safe (the theme value shouldn't contain executable code), it's worth noting that the content should never include unsanitized user input.Consider verifying whether the flicker is truly imperceptible in practice across browsers, and potentially qualify the claim or suggest the alternative
<head>placement for guidance completeness.
80-80: Clarify the timing description.The statement "executes synchronously before showing the element" is technically inaccurate. The script executes synchronously when the browser encounters the
<script>tag during HTML parsing, which is after the<div>has already been added to the DOM (since the script comes after the closing</div>tag).A more accurate description would be: "The inline script executes synchronously during HTML parsing, before React hydration begins, ensuring the DOM has the correct className when React takes over."
.opencode/skill/react-best-practices/rules/rendering-animate-svg-wrapper.md (3)
1-6: Reconsider the impact rating.The impact is marked as "LOW", but enabling hardware acceleration typically has a noticeable effect on animation performance and smoothness, particularly on mobile devices and lower-end hardware. Consider rating this as "MEDIUM" impact.
12-27: Add context about theanimate-spinclass.The example uses
animate-spinwithout explanation. Consider adding a note that this is a Tailwind CSS utility class, or provide a CSS definition for readers unfamiliar with Tailwind CSS. This would make the documentation more self-contained.
47-47: Consider clarifying CSS property references.The list includes both
transformand individual properties liketranslate,scale, androtate. In modern CSS,translate,scale, androtatecan be used as individual properties or astransformfunctions. Consider clarifying whether this refers to thetransformproperty with these functions (e.g.,transform: rotate(45deg)) or the individual CSS properties (e.g.,rotate: 45deg)..opencode/skill/react-best-practices/rules/client-event-listeners.md (1)
38-66: Add guidance on callback stability to prevent performance issues.The
useKeyboardShortcutimplementation depends oncallback(line 55), which means the effect will re-run and update the Map whenever the callback reference changes. If developers pass inline functions or callbacks that aren't memoized withuseCallback, this will cause unnecessary operations on every render, reducing the effectiveness of this optimization.Consider adding a note recommending that callbacks passed to
useKeyboardShortcutshould be stabilized usinguseCallback, or add an example showing proper usage.Additionally, the subscribe function (lines 57-65) doesn't follow the standard
useSWRSubscriptionsignature, which expects(key, { next }) => unsubscribe. While this works, showing the correct signature (even with unused parameters like(_key, _options)) would be more pedagogically complete.📝 Enhanced example with callback stability guidance
Add a note before or after the correct example:
**Important:** Callbacks should be memoized to prevent unnecessary re-registrations: ```tsx function Profile() { const handleP = useCallback(() => { /* ... */ }, []) const handleK = useCallback(() => { /* ... */ }, []) useKeyboardShortcut('p', handleP) useKeyboardShortcut('k', handleK) }Alternatively, update the subscribe function to follow SWR conventions:
- useSWRSubscription('global-keydown', () => { + useSWRSubscription('global-keydown', (_key, _options) => { const handler = (e: KeyboardEvent) => { if (e.metaKey && keyCallbacks.has(e.key)) { keyCallbacks.get(e.key)!.forEach(cb => cb()) } } window.addEventListener('keydown', handler) return () => window.removeEventListener('keydown', handler) }).opencode/skill/react-best-practices/rules/js-combine-iterations.md (1)
1-32: Consider adding guidance on when to apply this optimization.The code examples are correct, but the trade-off between functional/readable code and performance isn't discussed. With "LOW-MEDIUM" impact, combining iterations may not be worthwhile for small arrays or non-critical paths.
Consider adding a note about when to apply this optimization:
- Large arrays (1000+ elements)
- Hot code paths (executed frequently)
- Performance-critical operations
For typical use cases with small arrays, the readability of separate
.filter()calls often outweighs the minor performance gain..opencode/skill/react-best-practices/rules/js-min-max-loop.md (1)
74-82: Optional: Rephrase "very large arrays" for clarity.Based on the static analysis hint, consider rephrasing for improved clarity and specificity:
📝 Suggested rewording
-This works for small arrays but can be slower for very large arrays due to spread operator limitations. Use the loop approach for reliability. +This works for small arrays but can be slower for arrays with thousands of elements due to spread operator limitations. Use the loop approach for reliability..opencode/skill/react-best-practices/rules/server-parallel-fetching.md (1)
54-79: Consider adding explanation of why the children prop enables parallelism.The children prop pattern is correct, but the mechanism might not be obvious to all readers. When
Sidebaris passed aschildrentoLayout, React instantiates it before callingLayout, allowing both components to start their async work concurrently.📝 Optional clarification
Add a brief explanation after line 54:
**Alternative with children prop:** + +The `children` prop enables parallelism because React instantiates child components before the parent executes, allowing concurrent data fetching..opencode/skill/react-best-practices/rules/rendering-hoist-jsx.md (1)
14-42: Consider clarifying the comparison for better understanding.The current example contrasts a component function approach with a hoisted constant. While technically correct, readers might benefit from a more direct comparison showing inline JSX creation vs. hoisted JSX creation within the same component. However, given the LOW impact rating and the React Compiler note (line 46) that auto-optimizes this pattern, the current example is acceptable.
Alternative example structure for clarity
**Incorrect (recreates element every render):** ```tsx function Container() { + const loadingSkeleton = ( + <div className="animate-pulse h-20 bg-gray-200" /> + ) return ( <div> - {loading && <LoadingSkeleton />} + {loading && loadingSkeleton} </div> ) }This makes it clearer that the issue is element recreation vs. reuse, though your current version also effectively demonstrates the optimization. </details> </blockquote></details> <details> <summary>.opencode/skill/react-best-practices/README.md (1)</summary><blockquote> `58-84`: **Consider minor documentation polish (optional).** The rule template structure is clear and functional. If you'd like to enhance it slightly, consider: 1. Line 84: Using a more descriptive link text than "Link" in the template example (e.g., "Official documentation") 2. Line 122: The phrase "Originally created" could be simplified to "Created" These are purely stylistic improvements based on linting suggestions. </details> </blockquote></details> <details> <summary>.opencode/skill/react-best-practices/AGENTS.md (1)</summary><blockquote> `324-333`: **Add import statement and explanation for `use()` hook.** The `use()` hook is used here without an import statement or explanation of where it comes from. For AI agents consuming this documentation, adding `import { use } from 'react'` and noting that this requires React 19+ would improve clarity. <details> <summary>📝 Suggested addition</summary> Before the code example, add: ```tsx import { use } from 'react' // React 19+ function DataDisplay({ dataPromise }: { dataPromise: Promise<Data> }) { const data = use(dataPromise) // Unwraps the promise return <div>{data.content}</div> }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
.opencode/skill/react-best-practices/AGENTS.md.opencode/skill/react-best-practices/README.md.opencode/skill/react-best-practices/SKILL.md.opencode/skill/react-best-practices/rules/_sections.md.opencode/skill/react-best-practices/rules/_template.md.opencode/skill/react-best-practices/rules/advanced-event-handler-refs.md.opencode/skill/react-best-practices/rules/advanced-use-latest.md.opencode/skill/react-best-practices/rules/async-api-routes.md.opencode/skill/react-best-practices/rules/async-defer-await.md.opencode/skill/react-best-practices/rules/async-dependencies.md.opencode/skill/react-best-practices/rules/async-parallel.md.opencode/skill/react-best-practices/rules/async-suspense-boundaries.md.opencode/skill/react-best-practices/rules/bundle-barrel-imports.md.opencode/skill/react-best-practices/rules/bundle-conditional.md.opencode/skill/react-best-practices/rules/bundle-defer-third-party.md.opencode/skill/react-best-practices/rules/bundle-dynamic-imports.md.opencode/skill/react-best-practices/rules/bundle-preload.md.opencode/skill/react-best-practices/rules/client-event-listeners.md.opencode/skill/react-best-practices/rules/client-swr-dedup.md.opencode/skill/react-best-practices/rules/js-batch-dom-css.md.opencode/skill/react-best-practices/rules/js-cache-function-results.md.opencode/skill/react-best-practices/rules/js-cache-property-access.md.opencode/skill/react-best-practices/rules/js-cache-storage.md.opencode/skill/react-best-practices/rules/js-combine-iterations.md.opencode/skill/react-best-practices/rules/js-early-exit.md.opencode/skill/react-best-practices/rules/js-hoist-regexp.md.opencode/skill/react-best-practices/rules/js-index-maps.md.opencode/skill/react-best-practices/rules/js-length-check-first.md.opencode/skill/react-best-practices/rules/js-min-max-loop.md.opencode/skill/react-best-practices/rules/js-set-map-lookups.md.opencode/skill/react-best-practices/rules/js-tosorted-immutable.md.opencode/skill/react-best-practices/rules/rendering-activity.md.opencode/skill/react-best-practices/rules/rendering-animate-svg-wrapper.md.opencode/skill/react-best-practices/rules/rendering-conditional-render.md.opencode/skill/react-best-practices/rules/rendering-content-visibility.md.opencode/skill/react-best-practices/rules/rendering-hoist-jsx.md.opencode/skill/react-best-practices/rules/rendering-hydration-no-flicker.md.opencode/skill/react-best-practices/rules/rendering-svg-precision.md.opencode/skill/react-best-practices/rules/rerender-defer-reads.md.opencode/skill/react-best-practices/rules/rerender-dependencies.md.opencode/skill/react-best-practices/rules/rerender-derived-state.md.opencode/skill/react-best-practices/rules/rerender-functional-setstate.md.opencode/skill/react-best-practices/rules/rerender-lazy-state-init.md.opencode/skill/react-best-practices/rules/rerender-memo.md.opencode/skill/react-best-practices/rules/rerender-transitions.md.opencode/skill/react-best-practices/rules/server-after-nonblocking.md.opencode/skill/react-best-practices/rules/server-cache-lru.md.opencode/skill/react-best-practices/rules/server-cache-react.md.opencode/skill/react-best-practices/rules/server-parallel-fetching.md.opencode/skill/react-best-practices/rules/server-serialization.md
🧰 Additional context used
📓 Path-based instructions (1)
!src/components/routeTree.gen.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never hand-edit
routeTree.gen.ts; it is generated by TanStack Router tooling
Files:
.opencode/skill/react-best-practices/rules/rerender-dependencies.md.opencode/skill/react-best-practices/rules/rendering-conditional-render.md.opencode/skill/react-best-practices/rules/js-set-map-lookups.md.opencode/skill/react-best-practices/rules/rerender-memo.md.opencode/skill/react-best-practices/rules/rerender-functional-setstate.md.opencode/skill/react-best-practices/rules/async-suspense-boundaries.md.opencode/skill/react-best-practices/rules/js-early-exit.md.opencode/skill/react-best-practices/rules/server-parallel-fetching.md.opencode/skill/react-best-practices/rules/bundle-dynamic-imports.md.opencode/skill/react-best-practices/SKILL.md.opencode/skill/react-best-practices/rules/rendering-animate-svg-wrapper.md.opencode/skill/react-best-practices/rules/bundle-defer-third-party.md.opencode/skill/react-best-practices/rules/client-swr-dedup.md.opencode/skill/react-best-practices/rules/js-cache-storage.md.opencode/skill/react-best-practices/rules/rerender-defer-reads.md.opencode/skill/react-best-practices/rules/rendering-svg-precision.md.opencode/skill/react-best-practices/rules/rerender-derived-state.md.opencode/skill/react-best-practices/rules/js-cache-function-results.md.opencode/skill/react-best-practices/rules/js-tosorted-immutable.md.opencode/skill/react-best-practices/rules/rendering-activity.md.opencode/skill/react-best-practices/rules/js-combine-iterations.md.opencode/skill/react-best-practices/rules/bundle-preload.md.opencode/skill/react-best-practices/rules/rendering-hoist-jsx.md.opencode/skill/react-best-practices/README.md.opencode/skill/react-best-practices/rules/rendering-content-visibility.md.opencode/skill/react-best-practices/rules/server-cache-lru.md.opencode/skill/react-best-practices/rules/js-cache-property-access.md.opencode/skill/react-best-practices/rules/bundle-conditional.md.opencode/skill/react-best-practices/rules/js-hoist-regexp.md.opencode/skill/react-best-practices/rules/bundle-barrel-imports.md.opencode/skill/react-best-practices/rules/js-index-maps.md.opencode/skill/react-best-practices/rules/rendering-hydration-no-flicker.md.opencode/skill/react-best-practices/rules/advanced-event-handler-refs.md.opencode/skill/react-best-practices/rules/rerender-lazy-state-init.md.opencode/skill/react-best-practices/rules/async-dependencies.md.opencode/skill/react-best-practices/rules/js-min-max-loop.md.opencode/skill/react-best-practices/rules/server-after-nonblocking.md.opencode/skill/react-best-practices/AGENTS.md.opencode/skill/react-best-practices/rules/js-length-check-first.md.opencode/skill/react-best-practices/rules/server-serialization.md.opencode/skill/react-best-practices/rules/client-event-listeners.md.opencode/skill/react-best-practices/rules/_template.md.opencode/skill/react-best-practices/rules/rerender-transitions.md.opencode/skill/react-best-practices/rules/js-batch-dom-css.md.opencode/skill/react-best-practices/rules/_sections.md.opencode/skill/react-best-practices/rules/server-cache-react.md.opencode/skill/react-best-practices/rules/async-defer-await.md.opencode/skill/react-best-practices/rules/async-api-routes.md.opencode/skill/react-best-practices/rules/async-parallel.md.opencode/skill/react-best-practices/rules/advanced-use-latest.md
🪛 LanguageTool
.opencode/skill/react-best-practices/README.md
[style] ~122-~122: This phrase is redundant. Consider writing “created”.
Context: ...to manage numbers! ## Acknowledgments Originally created by @shuding at...
(ORIGINALLY_CREATED)
.opencode/skill/react-best-practices/rules/bundle-barrel-imports.md
[grammar] ~12-~12: Ensure spelling is correct
Context: ...For many React packages, it takes 200-800ms just to import them, affecting both d...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
.opencode/skill/react-best-practices/rules/js-min-max-loop.md
[style] ~82-~82: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... for small arrays but can be slower for very large arrays due to spread operator limitatio...
(EN_WEAK_ADJECTIVE)
.opencode/skill/react-best-practices/AGENTS.md
[grammar] ~359-~359: Ensure spelling is correct
Context: ...l File Imports Impact: CRITICAL (200-800ms import cost, slow builds) Import dir...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~817-~817: Use a hyphen to join words.
Context: ...tomatic deduplication and efficient data fetching patterns reduce redundant netwo...
(QB_NEW_EN_HYPHEN)
[style] ~2071-~2071: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... for small arrays but can be slower for very large arrays due to spread operator limitatio...
(EN_WEAK_ADJECTIVE)
[grammar] ~2194-~2194: Ensure spelling is correct
Context: ...latest version of the handler. ### 8.2 useLatest for Stable Callback Refs **Impact: LOW...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
.opencode/skill/react-best-practices/rules/_sections.md
[grammar] ~26-~26: Use a hyphen to join words.
Context: ...tomatic deduplication and efficient data fetching patterns reduce redundant netwo...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
.opencode/skill/react-best-practices/SKILL.md
111-111: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
.opencode/skill/react-best-practices/README.md
76-76: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
76-76: Strong style
Expected: underscore; Actual: asterisk
(MD050, strong-style)
84-84: Link text should be descriptive
(MD059, descriptive-link-text)
.opencode/skill/react-best-practices/AGENTS.md
81-81: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
87-87: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
107-107: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
122-122: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
163-163: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
167-167: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
177-177: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
195-195: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
199-199: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
210-210: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
229-229: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
233-233: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
241-241: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
253-253: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
257-257: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
278-278: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
304-304: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
353-353: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
359-359: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
367-367: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
378-378: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
391-391: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
414-414: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
418-418: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
441-441: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
445-445: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
462-462: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
486-486: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
490-490: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
500-500: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
517-517: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
521-521: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
543-543: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
565-565: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
571-571: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
608-608: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
612-612: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
626-626: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
642-642: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
646-646: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
665-665: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
717-717: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
739-739: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
743-743: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
763-763: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
815-815: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
821-821: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
825-825: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
843-843: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
891-891: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
895-895: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
908-908: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
945-945: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
951-951: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
955-955: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
970-970: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
986-986: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
990-990: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1004-1004: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1026-1026: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1030-1030: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1038-1038: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1067-1067: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1071-1071: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1081-1081: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1092-1092: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1096-1096: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1118-1118: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1170-1170: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1174-1174: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1196-1196: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1224-1224: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1228-1228: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1241-1241: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1262-1262: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1268-1268: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1272-1272: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1289-1289: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1311-1311: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1345-1345: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1349-1349: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1365-1365: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1387-1387: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1391-1391: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1397-1397: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1411-1411: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1415-1415: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1432-1432: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1456-1456: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1489-1489: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1511-1511: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1515-1515: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1530-1530: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1549-1549: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1555-1555: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1559-1559: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1571-1571: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1588-1588: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1633-1633: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1667-1667: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1671-1671: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1679-1679: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1691-1691: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1695-1695: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1712-1712: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1767-1767: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1771-1771: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1780-1780: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1815-1815: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1833-1833: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1837-1837: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1845-1845: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1861-1861: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1867-1867: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1910-1910: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1914-1914: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1937-1937: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1956-1956: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1960-1960: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1970-1970: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1985-1985: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1997-1997: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2063-2063: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2075-2075: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2095-2095: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2099-2099: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2112-2112: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2131-2131: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2154-2154: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2160-2160: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2164-2164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2175-2175: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2196-2196: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2212-2212: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
2225-2225: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
.opencode/skill/react-best-practices/rules/_template.md
10-10: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| **Impact: MEDIUM (faster response times)** | ||
|
|
||
| Use Next.js's `after()` to schedule work that should execute after a response is sent. This prevents logging, analytics, and other side effects from blocking the response. | ||
|
|
||
| **Incorrect: blocks response** | ||
|
|
||
| ```tsx | ||
| import { logUserAction } from '@/app/utils' | ||
|
|
||
| export async function POST(request: Request) { | ||
| // Perform mutation | ||
| await updateDatabase(request) | ||
|
|
||
| // Logging blocks the response | ||
| const userAgent = request.headers.get('user-agent') || 'unknown' | ||
| await logUserAction({ userAgent }) | ||
|
|
||
| return new Response(JSON.stringify({ status: 'success' }), { | ||
| status: 200, | ||
| headers: { 'Content-Type': 'application/json' } | ||
| }) | ||
| } | ||
| ``` | ||
|
|
||
| **Correct: non-blocking** | ||
|
|
||
| ```tsx | ||
| import { after } from 'next/server' | ||
| import { headers, cookies } from 'next/headers' | ||
| import { logUserAction } from '@/app/utils' | ||
|
|
||
| export async function POST(request: Request) { | ||
| // Perform mutation | ||
| await updateDatabase(request) | ||
|
|
||
| // Log after response is sent | ||
| after(async () => { | ||
| const userAgent = (await headers()).get('user-agent') || 'unknown' | ||
| const sessionCookie = (await cookies()).get('session-id')?.value || 'anonymous' | ||
|
|
||
| logUserAction({ sessionCookie, userAgent }) | ||
| }) | ||
|
|
||
| return new Response(JSON.stringify({ status: 'success' }), { | ||
| status: 200, | ||
| headers: { 'Content-Type': 'application/json' } | ||
| }) | ||
| } | ||
| ``` | ||
|
|
||
| The response is sent immediately while logging happens in the background. | ||
|
|
||
| **Common use cases:** | ||
|
|
||
| - Analytics tracking | ||
|
|
||
| - Audit logging | ||
|
|
||
| - Sending notifications | ||
|
|
||
| - Cache invalidation | ||
|
|
||
| - Cleanup tasks | ||
|
|
||
| **Important notes:** | ||
|
|
||
| - `after()` runs even if the response fails or redirects | ||
|
|
||
| - Works in Server Actions, Route Handlers, and Server Components | ||
|
|
||
| Reference: [https://nextjs.org/docs/app/api-reference/functions/after](https://nextjs.org/docs/app/api-reference/functions/after) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Next.js after() function version requirement when introduced
💡 Result:
The after() API was introduced as unstable_after in the Next.js 15.0.0-rc release and became stable as after in Next.js v15.1.0. [1][2]
Sources:
[1] Next.js docs — Functions: after (version history).
[2] Next.js international docs — after (version history).
Add Next.js version requirement for after() function.
The after() API became stable in Next.js v15.1.0. The documentation snippet should specify this minimum version requirement so users know whether they can use this pattern in their projects.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
739-739: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
743-743: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
763-763: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/AGENTS.md around lines 738 - 809, Add a
clear minimum Next.js version note for the after() API by updating the AGENTS.md
content around the after() examples and the "Important notes" section to state
that after() became stable in Next.js v15.1.0 (include "Next.js v15.1.0+" or
similar), so readers know the feature availability; reference the after() symbol
in the text and the Next.js docs link already present to guide users.
| ### 6.6 Use Activity Component for Show/Hide | ||
|
|
||
| **Impact: MEDIUM (preserves state/DOM)** | ||
|
|
||
| Use React's `<Activity>` to preserve state/DOM for expensive components that frequently toggle visibility. | ||
|
|
||
| **Usage:** | ||
|
|
||
| ```tsx | ||
| import { Activity } from 'react' | ||
|
|
||
| function Dropdown({ isOpen }: Props) { | ||
| return ( | ||
| <Activity mode={isOpen ? 'visible' : 'hidden'}> | ||
| <ExpensiveMenu /> | ||
| </Activity> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| Avoids expensive re-renders and state loss. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
React Activity component experimental canary stable React 18 React 19
💡 Result:
Short answer: The new Activity component started as an experimental/Canary feature and is now shipped in React 19.2 (stable). [1][2]
Details:
- Announced as experimental / available in react@experimental and react@canary (previously called Offscreen → renamed Activity). [1][3]
- Marked as shipped in [email protected] (React 19 stable channel) and documented for use (modes 'visible' and 'hidden'). [1][2]
Sources:
- React Labs blog: View Transitions, Activity, and more — notes Activity shipped in [email protected] and available in experimental/canary. [1]
- React 19 release notes (react.dev blog). [2]
- Coverage: InfoQ summary of React 19.2 Activity API. [3]
Add React version requirement: Activity shipped in React 19.2.
The <Activity> component is now stable and shipped in React 19.2, so it is a legitimate API. However, the documentation should specify the minimum React version required, as developers targeting React 18 or earlier would encounter import errors.
Consider adding a note like: "Available in React 19.2+".
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1489-1489: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/AGENTS.md around lines 1487 - 1507,
Update the guidance for using the Activity component to state the minimum React
version required: mention that the Activity component (import { Activity } from
'react') is available starting in React 19.2+, so consumers on React 18 or
earlier will get import errors; add a short note or parenthetical next to the
Usage/heading such as "Available in React 19.2+" and ensure references to the
Activity symbol remain unchanged.
| ```typescript | ||
| function useLatest<T>(value: T) { | ||
| const ref = useRef(value) | ||
| useEffect(() => { | ||
| ref.current = value | ||
| }, [value]) | ||
| return ref | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider updating ref synchronously during render.
The useLatest implementation updates the ref in useEffect, which runs after render commits. The standard pattern updates ref.current directly during render to ensure the ref always holds the latest value without any timing gap.
♻️ Standard useLatest pattern
function useLatest<T>(value: T) {
const ref = useRef(value)
- useEffect(() => {
- ref.current = value
- }, [value])
+ ref.current = value
return ref
}This ensures callbacks always see the latest value, even if invoked between render and effect execution.
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/advanced-use-latest.md around
lines 14 - 22, The current useLatest updates ref.current in useEffect which can
lag; modify the useLatest function so it sets ref.current = value synchronously
during render (i.e., immediately after creating ref with useRef) and remove the
useEffect (or keep it only if you must for some edge case), ensuring the
returned ref from useLatest always contains the latest value for callers;
reference the useLatest function, the ref variable from useRef, and the current
useEffect invocation when making the change.
|
|
||
| ## Dependency-Based Parallelization | ||
|
|
||
| For operations with partial dependencies, use `better-all` to maximize parallelism. It automatically starts each task at the earliest possible moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Is the better-all npm package actively maintained and what is its current version?
💡 Result:
I can't find a package named "better-all" on npm. Could you confirm the exact package name (or paste its npm/GitHub link) so I can check its version and maintenance status?
🏁 Script executed:
#!/bin/bash
# Description: Check if better-all is already used in the project
# Search for better-all in package files
fd -t f "package.json" --exec jq -r 'select(.dependencies."better-all" or .devDependencies."better-all") | {"better-all": (.dependencies."better-all" // .devDependencies."better-all")}'
# Search for existing imports
rg -n "from ['\"]better-all['\"]" --type ts --type tsx --type js --type jsxRepository: ls1intum/Hephaestus
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for better-all in package files
fd -t f "package.json" --exec jq -r 'select(.dependencies."better-all" or .devDependencies."better-all") | {"better-all": (.dependencies."better-all" // .devDependencies."better-all")}'
# Search for existing imports with corrected file types
rg -n "better-all" -t json -t js -t ts
# Also search for similar patterns in markdown documentation
rg -n "better-all|betterAll|better_all" .opencode/Repository: ls1intum/Hephaestus
Length of output: 1748
🌐 Web query:
npm package "better-all" OR "better_all" OR "betterAll" dependency parallelization
💡 Result:
I couldn't find an npm package named better-all, better_all, or betterAll. If you meant a utility to run promises/functions in parallel with optional concurrency limits, common alternatives are:
- p-all — run functions that return promises with concurrency control. [1]
- p-parallel — run promise-returning functions in parallel with a limit. [2]
- batched-promise-all — run async work in batches to keep memory usage low. [3]
If you want, tell me which API you prefer (Promise-array style or function-array style) and I’ll show example usage or search deeper.
Sources:
[1] p-all (npm).
[2] p-parallel (package page / Socket).
[3] batched-promise-all (npm).
🌐 Web query:
GitHub better-all parallelization task dependencies
💡 Result:
Do you mean GitHub Actions (workflows) and how to parallelize jobs/tasks that have dependencies, or something else (a specific repo/action named "better-all")?
🌐 Web query:
GitHub shuding better-all repository maintenance status
💡 Result:
I can't find a clear GitHub page for "shuding/better-all" from public searches. Could you paste the repository URL (or confirm the exact owner/name) so I can check its maintenance status (last commit, issues/PR activity, releases, etc.)?
🌐 Web query:
shuding/better-all JavaScript library async parallelization
💡 Result:
I found shuding's "better-all" as a small library that provides a "Better Promise.all" — automatic dependency optimization + full TypeScript type inference to run async tasks more ergonomically (lets fields be awaited optimally instead of manual Promise.all chains). Source: Shu (shuding) announcement / tweet referencing the GitHub repo. [1]
If you want I can:
- open the repo and summarise API + examples, or
- show how to achieve similar behavior with plain Promise utilities (Promise.all, Promise.allSettled, concurrency limits), or
- fetch README/docs and usage examples from the repository. Which would you like?
🌐 Web query:
shuding better-all npm package install
💡 Result:
If the package is published to npm
- npm: npm install better-all
- yarn: yarn add better-all
If it’s not on npm but the source is on GitHub (shuding/better-all), install directly from the repo:
- npm: npm install github:shuding/better-all
- Yarn: yarn add https://github.com/shuding/better-all.git
(You can also point npm at a specific branch, tag or commit: npm install github:shuding/better-all#main). Source: GitHub Packages docs (install from a repo). [1][2]
Sources:
- GitHub — Installing a package (GitHub Packages). [2]
- Tweet referencing shuding's better-all repo. [1]
🌐 Web query:
"better-all" JavaScript Promise library how to install
💡 Result:
I couldn't find a JavaScript Promise library named "better-all" on npm or GitHub. Could you share a link or the exact package name?
If you meant a regular npm package, install it with:
- npm: npm install . (docs.npmjs.com)
Or with yarn:
- yarn: yarn add . (classic.yarnpkg.com)
Citations:
- 1: https://docs.npmjs.com/cli/v9/commands/npm-install?utm_source=openai
- 2: https://classic.yarnpkg.com/lang/en/docs/cli/add/?utm_source=openai
The better-all library is not available on npm and its GitHub repository is not publicly discoverable—this is a significant issue for documentation recommending it.
Web searches confirm that better-all is not published as an npm package. While the library appears to exist as a GitHub project by Shu Ding, the repository is not publicly accessible or findable via standard searches, making it impossible to verify:
- Current maintenance status and activity
- API syntax accuracy (e.g.,
await this.$.user) - Security posture or known vulnerabilities
Recommending a library not on npm contradicts standard dependency management practices. Consider either:
- Removing the recommendation if the library is not suitable for mainstream use
- Using a well-maintained, publicly available alternative (e.g.,
p-all,p-queue) - If the library is intentionally private/niche, clarify its status and installation requirements in the documentation
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/async-dependencies.md at line 10,
The doc recommends the unfindable "better-all" library; fix this by removing or
replacing that recommendation and clarifying install/status: either delete the
"better-all" mention in the rule text, or swap it for a public, maintained
alternative such as "p-all" or "p-queue" and update example usages (e.g., the
`better-all` reference and `await this.$.user` example) to match the chosen
library's API; if "better-all" must remain, add a clear note about its private
status and explicit installation/source instructions and maintenance caveats.
| function DataDisplay({ dataPromise }: { dataPromise: Promise<Data> }) { | ||
| const data = use(dataPromise) // Unwraps the promise | ||
| return <div>{data.content}</div> | ||
| } | ||
|
|
||
| function DataSummary({ dataPromise }: { dataPromise: Promise<Data> }) { | ||
| const data = use(dataPromise) // Reuses the same promise | ||
| return <div>{data.summary}</div> | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
React use hook API promise unwrapping React 19 documentation
💡 Result:
Short summary — React 19 "use" hook (promise unwrapping)
-
What it does: import { use } from 'react' lets a component read a resource (Promise or Context) directly in render; when given a Promise, React will suspend the component until the Promise resolves and then return the resolved value. [1][2]
-
Syntax example:
const value = use(somePromise); // suspends until somePromise resolves. [1][2] -
Key behaviors and caveats:
- Integrates with Suspense and Error Boundaries (shows Suspense fallback while pending; rejected promises surface to nearest Error Boundary). [2]
- Unlike regular hooks, use() may be called conditionally or in loops (but must be called from a Component or Hook). [2]
- use() does not support promises created during the same render in Client Components — passing such uncached promises will warn; prefer passing stable promises (e.g., from Server Components or a Suspense-aware cache). [1][2]
- For Server Components, prefer async/await over use() in many cases (async/await picks up rendering at await points; use() triggers a re-render after resolution). [2]
Sources:
- React 19 blog/feature announcement. [1]
- React v19 API reference for use. [2]
🏁 Script executed:
#!/bin/bash
# Check if there's a package.json or similar that specifies React version
git ls-files | grep -E "(package\.json|package-lock|yarn\.lock|pnpm-lock|tsconfig|README)" | head -20Repository: ls1intum/Hephaestus
Length of output: 598
🏁 Script executed:
#!/bin/bash
# Check root package.json and docs/package.json for React version
echo "=== Root package.json ==="
cat package.json | grep -A2 '"react"'
echo -e "\n=== docs/package.json ==="
cat docs/package.json | grep -A2 '"react"'Repository: ls1intum/Hephaestus
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Read the documentation file to see if React version is already mentioned
cat -n .opencode/skill/react-best-practices/rules/async-suspense-boundaries.mdRepository: ls1intum/Hephaestus
Length of output: 3265
Add React version requirement documentation for the use hook.
The use hook implementation is correct, but the documentation should note that this requires React 19+. Developers should know that the alternative pattern (lines 79–87) won't work in earlier versions. Add a brief note or version badge to clarify this prerequisite.
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/async-suspense-boundaries.md
around lines 79 - 87, Add a short note stating that the example using the use
hook requires React 19+ so it will not work in earlier React versions; update
the documentation near the DataDisplay and DataSummary examples to include a
brief version badge or one-line sentence like “Requires React 19+ (use hook
support)” so readers know the prerequisite for using use with promises.
| function Highlighter({ text, query }: Props) { | ||
| const regex = useMemo( | ||
| () => new RegExp(`(${escapeRegex(query)})`, 'gi'), | ||
| [query] | ||
| ) | ||
| const parts = text.split(regex) | ||
| return <>{parts.map((part, i) => ...)}</> | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define or explain the escapeRegex utility function.
The example references escapeRegex(query) without defining or explaining it. This is important because:
- Security/Correctness: Without escaping, special regex characters in
query(like.,*,?,[,], etc.) will break the RegExp or cause unexpected matching behavior. - Developer guidance: Readers implementing this pattern won't know they need to sanitize the input or how to do it.
📝 Suggested additions
Add the helper function definition or at minimum add a comment explaining it:
function Highlighter({ text, query }: Props) {
+ // escapeRegex should escape special regex characters: . * + ? ^ $ { } ( ) | [ ] \
const regex = useMemo(
() => new RegExp(`(${escapeRegex(query)})`, 'gi'),
[query]
)Or provide the implementation at the top of the example:
+function escapeRegex(str: string): string {
+ return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+}
+
const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/📝 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.
| function Highlighter({ text, query }: Props) { | |
| const regex = useMemo( | |
| () => new RegExp(`(${escapeRegex(query)})`, 'gi'), | |
| [query] | |
| ) | |
| const parts = text.split(regex) | |
| return <>{parts.map((part, i) => ...)}</> | |
| } | |
| ``` | |
| function Highlighter({ text, query }: Props) { | |
| // escapeRegex should escape special regex characters: . * + ? ^ $ { } ( ) | [ ] \ | |
| const regex = useMemo( | |
| () => new RegExp(`(${escapeRegex(query)})`, 'gi'), | |
| [query] | |
| ) | |
| const parts = text.split(regex) | |
| return <>{parts.map((part, i) => ...)}</> | |
| } |
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/js-hoist-regexp.md around lines
27 - 35, The snippet uses escapeRegex(query) but doesn't define it; add a safe
helper named escapeRegex and use it when building the RegExp in Highlighter (the
useMemo that constructs regex) so queries with special regex characters are
escaped; implement escapeRegex to escape characters like . * + ? ^ $ { } ( ) | [
] \ / and ensure Highlighter continues to call escapeRegex(query) before new
RegExp(...) to avoid broken or unsafe regexes.
| --- | ||
| title: CSS content-visibility for Long Lists | ||
| impact: HIGH | ||
| impactDescription: faster initial render | ||
| tags: rendering, css, content-visibility, long-lists | ||
| --- | ||
|
|
||
| ## CSS content-visibility for Long Lists | ||
|
|
||
| Apply `content-visibility: auto` to defer off-screen rendering. | ||
|
|
||
| **CSS:** | ||
|
|
||
| ```css | ||
| .message-item { | ||
| content-visibility: auto; | ||
| contain-intrinsic-size: 0 80px; | ||
| } | ||
| ``` | ||
|
|
||
| **Example:** | ||
|
|
||
| ```tsx | ||
| function MessageList({ messages }: { messages: Message[] }) { | ||
| return ( | ||
| <div className="overflow-y-auto h-screen"> | ||
| {messages.map(msg => ( | ||
| <div key={msg.id} className="message-item"> | ||
| <Avatar user={msg.author} /> | ||
| <div>{msg.content}</div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| For 1000 messages, browser skips layout/paint for ~990 off-screen items (10× faster initial render). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
CSS content-visibility browser support compatibility 2024 2025
💡 Result:
Short summary — support in 2024–2025
- Broad support in modern browsers: Chrome/Chromium (since v85), Edge (since v85), Opera (since ~v71). [1]
- Firefox: behind at first (disabled-by-default in ~v109–124), full support landed around v125+ (2024). [1][2]
- Safari / WebKit: rollout later — WebKit/Safari added support in 2024–2025 (Web.dev notes “available in all three major browser engines” and marks the feature Baseline Newly available as of Sep 15, 2025). Some older Safari/iOS versions remain unsupported. [3][1]
- Practical guidance: content-visibility is production-ready for most modern users, but test for older Firefox builds (pre-125 or where it was disabled) and older Safari/iOS versions; use feature-detection or fallbacks if you must support those. [1][2]
Sources: Can I Use (CSS content-visibility) [1], MDN content-visibility [2], web.dev article / Baseline status [3].
Update compatibility note to reflect current browser support.
The CSS content-visibility property is now widely supported across all major browser engines (Chrome/Chromium v85+, Firefox v125+, Safari/WebKit 2024–2025). Adding a note about browser compatibility remains a good practice, but clarify that it has broad modern browser support rather than "limited support." For older browser versions (pre-v125 Firefox, older Safari), consider documenting the fallback gracefully degrades (e.g., rendering without the performance optimization).
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/rendering-content-visibility.md
around lines 1 - 38, Update the compatibility note in the
rendering-content-visibility rule to state that content-visibility is now
broadly supported across major engines (Chrome/Chromium v85+, Firefox v125+,
Safari/WebKit 2024–2025) instead of implying limited support; in the doc
(title/description and the example block referencing .message-item and the
MessageList component) add a short sentence advising graceful fallback for older
browsers (pre-v125 Firefox, older Safari) such as rendering normally when the
property is unsupported and optionally feature-detect via CSS.supports; keep the
CSS example for .message-item and the MessageList example unchanged but append
the compatibility/fallback note so readers know when and how to degrade.
| const userAgent = (await headers()).get('user-agent') || 'unknown' | ||
| const sessionCookie = (await cookies()).get('session-id')?.value || 'anonymous' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for package.json files and extract Next.js version
fd -t f "package.json" --exec sh -c 'echo "File: $1"; cat "$1" | head -50' _ {} \; | head -100Repository: ls1intum/Hephaestus
Length of output: 6394
🌐 Web query:
When did Next.js introduce async headers() and cookies() API
💡 Result:
- Introduced: headers() and cookies() were introduced in Next.js v13.0.0 (Next.js 13 release on October 25, 2022). [1][2][3]
- Made asynchronous: the APIs were changed to be async as part of the Next.js 15 changes (announced in the v15 canary/RC work — e.g. v15.0.0-canary.171 / related breaking-change guidance in Oct 2024). [4][2]
Sources:
- cookies() doc (version history: v13.0.0 introduced). [1]
- headers() doc (version history: v13.0.0 introduced; later async change noted in v15 docs). [2]
- Next.js 13 release announcement (Oct 25, 2022). [3]
- Next.js GitHub discussion and notes about making dynamic APIs async in v15 (Oct 2024). [4]
🏁 Script executed:
#!/bin/bash
# Extract Next.js version from webapp package.json
cat ./webapp/package.json | grep -A 50 "dependencies" | grep -E "next|react"Repository: ls1intum/Hephaestus
Length of output: 1546
🏁 Script executed:
#!/bin/bash
# Get the full dependencies section from webapp/package.json
jq '.dependencies | keys | .[]' ./webapp/package.json | grep -i nextRepository: ls1intum/Hephaestus
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Read the review file to understand what it is
head -60 '.opencode/skill/react-best-practices/rules/server-after-nonblocking.md'Repository: ls1intum/Hephaestus
Length of output: 1701
🏁 Script executed:
#!/bin/bash
# Search for Next.js usage across the entire project
rg "next/server|next/headers|from 'next" --type ts --type tsx --type js --type jsx | head -20Repository: ls1intum/Hephaestus
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Also check what the .opencode directory is for
find .opencode -type f -name "*.md" | head -10 && echo "---" && ls -la .opencode/ 2>/dev/null | head -20Repository: ls1intum/Hephaestus
Length of output: 854
🏁 Script executed:
#!/bin/bash
# Check the skill README and other documentation
cat .opencode/skill/react-best-practices/README.mdRepository: ls1intum/Hephaestus
Length of output: 3424
🏁 Script executed:
#!/bin/bash
# Search for any Next.js references in the skill files
rg "Next.js|version" .opencode/skill/react-best-practices/ --type mdRepository: ls1intum/Hephaestus
Length of output: 2978
Add explicit Next.js version requirement to the rule.
The async headers() and cookies() API used in the code example is correct for Next.js 15+. However, the rule should explicitly state this version requirement, since earlier Next.js versions (13-14) use synchronous APIs. Add a note at the start of the rule or in the frontmatter clarifying that this pattern requires Next.js 15 or later.
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/server-after-nonblocking.md
around lines 45 - 46, The rule's example uses the async Next.js APIs headers()
and cookies(), which only exist in Next.js 15+; update the rule to explicitly
state the Next.js version requirement by adding a note or frontmatter indicating
this pattern requires Next.js 15 or later (and that Next.js 13–14 use
synchronous headers/cookies APIs), and mention the specific symbols headers()
and cookies() so readers know which APIs are version-dependent.
| ```typescript | ||
| import { cache } from 'react' | ||
|
|
||
| export const getCurrentUser = cache(async () => { | ||
| const session = await auth() | ||
| if (!session?.user?.id) return null | ||
| return await db.user.findUnique({ | ||
| where: { id: session.user.id } | ||
| }) | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
React.cache() API stability minimum version requirement 2024 2025
💡 Result:
Short answer: React.cache was introduced as an experimental/canary API (first appearing in React canary builds around the 18.3 canary/experimental channel) and remained marked non‑stable/Canary for Server Components through 2024–2025. It is not a guaranteed stable API in those years — use a React canary/experimental release (or frameworks that vendor an experimental React) to get it. [1][2][3]
Sources
- React cache docs (Canary/experimental note). [1]
- React docs showing cache in React v19.1 reference (Server Components usage). [2]
- Next.js discussion noting differences and when projects embed experimental React. [3]
References
- React reference: cache (canary/experimental). [turn0search2]
- React v19.1 cache docs (Server Components). [turn0search6]
- Next.js docs/issue discussing React.cache usage and framework bundling. [turn0search9]
Document React.cache() as experimental and clarify version requirements.
React.cache() is an experimental/canary API and not available in stable React releases. The code example is syntactically correct for Server Components, but the documentation should clearly note that this requires React's canary/experimental channel (or a framework that vendors experimental React) and may break between versions. Add a note indicating this is not production-stable in standard React releases.
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/rules/server-cache-react.md around
lines 14 - 24, Mention that React.cache (used in the getCurrentUser example) is
an experimental/canary-only API: update the text around the example to
explicitly state that React.cache is part of React's experimental/canary channel
(or only available when a framework vendors an experimental React), may be
absent in stable releases, and can change between versions; reference the symbol
React.cache and the example function getCurrentUser so readers know the
requirement and risk.
| ``` | ||
| rules/async-parallel.md | ||
| rules/bundle-barrel-imports.md | ||
| rules/_sections.md | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifier to fenced code block.
The code block listing rule file paths is missing a language identifier, flagged by markdownlint.
Based on static analysis hints (markdownlint-cli2).
📝 Proposed fix
-```
+```text
rules/async-parallel.md
rules/bundle-barrel-imports.md
rules/_sections.md
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
111-111: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @.opencode/skill/react-best-practices/SKILL.md around lines 111 - 115, The
fenced code block that lists rule file paths (the triple-backtick block
containing "rules/async-parallel.md", "rules/bundle-barrel-imports.md",
"rules/_sections.md") is missing a language identifier; update that block to use
a language tag (e.g., change ``` to ```text) so markdownlint recognizes it as a
plain-text fenced code block.
Description
This PR integrates the Vercel React Best Practices skill into the
.opencode/skilldirectory. This allows AI agents to access and follow comprehensive performance optimization guidelines for React and Next.js development within this project.How to test
Verify that
.opencode/skill/react-best-practices/SKILL.mdexists and contains the correct instructions. CI covers this as it is a documentation/configuration change.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.