feat: yield improvements p2 #11703
feat: yield improvements p2 #11703
Conversation
Remove titleOverride props so single yields show their metadata.name (e.g., "Aave v3 USDC Lending") instead of just the asset symbol. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Display warning badges on yield cards and alerts on detail pages when yields are marked as underMaintenance or deprecated in the API. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…badges Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create reusable YieldExplainers component that shows reward schedule, unbonding periods, and other yield-specific info based on mechanics type. Add it to YieldEnterModal and EarnConfirm for consistent user education across all enter flows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show a "Learn more" link with external icon when a yield has metadata.documentation available. Appears below the description text on the yield detail page. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add YieldExplainers component to YieldForm.tsx (yield page enter modal) - Update ProviderDto to include website and references fields - Use provider.references[0] for documentation link (protocol website) - Fix docs link styling: inline icon next to description - Prefer provider documentation over yield metadata documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 3-tab structure: All | Available to Earn | My Positions - Auto-navigate to "Available to Earn" tab when wallet connects - Each tab has explicit URL param for proper navigation - Keep recommended strip visible in all tabs - Add YieldTab enum for type safety - Rename Earn button action to navigate to Available tab Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix filters not working on My Positions tab (use unfiltered data) - Fix filters affecting Recommended For You (should be independent) - Fix "Show all" button navigating to All tab when on Available tab - Show full metadata.name on yield detail page below provider pill - Hide Withdraw button when user has no balance (instead of disabled) - Add YieldProviderInfo component with provider descriptions - Add YieldRelatedMarkets component showing other yields for same token - Make Yields first tab in Earn menu when feature flag enabled - Code simplification: remove unnecessary useMemo wrappers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show fiat value as primary (large text) and crypto as secondary. This matches user expectations where fiat is the more relevant metric. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a professional two-column layout for desktop screens with info on the left and actions on the right. Create YieldInfoCard and YieldAvailableToDeposit components. Mobile layout remains unchanged. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix validator mismatch on yield detail page by passing selectedValidatorAddress from YieldDetail to YieldPositionCard (ensures Cosmos uses ShapeShift DAO) - Add "New" badge support to NavigationDropdown component - Mark Yields menu item with New badge - Apply react-best-practices cleanup to yield components Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create getYieldDisplayName that returns curator name for Morpho/Yearn vaults (e.g., "Steakhouse High Yield") and symbol for simple yields - Integrate into YieldItem, YieldDetail, YieldInfoCard, YieldHero - Remove useless secondary row on mobile showing ugly metadata name - Fix Visit Website alignment in YieldProviderInfo - Fix Lending badge from UPPERCASE to Capitalize - Simplify components by removing unnecessary useMemo calls Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace cramped pill with clean icon + label pairs - Show Asset (icon + symbol), Chain (icon + name), Protocol (icon + name) - Apply same pattern to both desktop (YieldInfoCard) and mobile (YieldHero) - Remove redundant subtitle section - Clean up unused code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces comprehensive React best practices documentation (40+ performance optimization rules) and significantly expands the Yields feature with new UI components, utilities, translations, and navigation enhancements including provider information, explainers, and related market discovery. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
gomesalexandre
changed the title
| "quoteFailedDescription": "Unable to get a quote for this transaction. Please try again." | ||
| } | ||
| }, | ||
| "underMaintenance": "Under Maintenance", |
There was a problem hiding this comment.
revert ea4338b to test deprecated/under maintenance
| <NavigationDropdown | ||
| label='defi.earn' | ||
| items={earnSubMenuItems} | ||
| defaultPath={isYieldXyzEnabled ? '/yields' : '/tcy'} |
There was a problem hiding this comment.
Yield as the default path for Earn tab (assuming yield flag is on)
There was a problem hiding this comment.
Untranslated is on purpose for now!
There was a problem hiding this comment.
… skill - Prefer arrow functions over function keyword in memo components - Prefer useMemo over IIFE in TSX - Prefer implicit returns in useMemo/useCallback - Document avoidance of nested ternaries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In @.claude/skills/react-best-practices/references/rules/bundle-conditional.md:
- Around line 14-29: The error handler in AnimationPlayer references setEnabled
which doesn't exist; update the component to either manage its own enabled state
or change the error handling: add local state like const [isEnabled,
setIsEnabled] = useState(enabled) and use isEnabled in the effect and
setIsEnabled(false) in the catch, or remove the setEnabled call and handle the
error by logging or setting frames to an empty array (e.g., setFrames([]));
ensure all references in useEffect and the prop usage are updated to use the
chosen local symbol (isEnabled/setIsEnabled) or no longer call setEnabled.
In
@.claude/skills/react-best-practices/references/rules/client-event-listeners.md:
- Around line 57-66: The useSWRSubscription call is missing its closing
parenthesis and therefore the hook invocation isn't terminated; close the call
by adding the missing ")" after the hook's cleanup return so that the function
body for useSWRSubscription (which defines handler and uses keyCallbacks and
window.addEventListener/removeEventListener) is properly closed—ensure the final
lines read as the cleanup return followed by a closing parenthesis and any
necessary semicolon.
In
@.claude/skills/react-best-practices/references/rules/js-cache-property-access.md:
- Around line 12-18: The comment incorrectly states "(3 lookups × N iterations)"
while the loop also reads arr.length each iteration; update the comment near the
example so it either counts arr.length (e.g., "(4 lookups × N iterations)"
counting arr.length + obj.config.settings.value) or rephrase to explicitly list
both costs (e.g., "arr.length + obj.config.settings.value — total 4 property
accesses per iteration"); reference the loop variables (for, arr.length) and the
nested property access (obj.config.settings.value) when making the
clarification.
- Around line 20-28: Update the example comment and/or code to accurately
represent what's being optimized: change the label "(1 lookup total)" to
something like "Correct (cache lookups before the loop)" or similar, and either
remove the cached arr.length example or note that arr.length caching is an
edge-case optimization; specifically reference the nested access
obj.config.settings.value (which is being cached into value) and the arr.length
caching, and instruct to either keep only the nested property caching example or
add a clarifying note that arr.length caching is typically unnecessary in modern
engines except for polymorphic/mutable-array scenarios.
In @.claude/skills/react-best-practices/references/rules/js-cache-storage.md:
- Around line 41-54: The cookie parsing in getCookie (using
document.cookie.split('; ').map(c => c.split('='))) breaks on values with '='
and doesn't decode or validate entries; fix by parsing each cookie string by
finding the first '=' (e.g., indexOf) to split name and value, decode the value
with decodeURIComponent, ignore empty/malformed entries, and keep the
cookieCache logic intact so getCookie(name) returns the properly decoded value
or undefined for missing/invalid cookies.
In
@.claude/skills/react-best-practices/references/rules/js-length-check-first.md:
- Around line 45-49: Clarify which optimization yields each benefit by mapping
the bullet points to the specific code changes: state that the length-check
(comparing array.length) provides the early-return for differing sizes; that
using .toSorted() versus .sort() prevents mutating the original arrays; that
avoiding join() and string comparisons removes extra memory allocation for large
arrays; and that the explicit element-by-element loop (or early-exit
for/for...of) enables returning early when a difference is found; alternatively,
update the title/description to say the example demonstrates multiple best
practices (length check, non-mutating sort, and early-exit comparison) so
readers understand each bullet corresponds to a distinct change (.length,
.toSorted()/ .sort(), join(), loop early-exit).
- Around line 34-35: Replace uses of the ES2023 .toSorted() calls by creating a
shallow copy and calling .sort() so the code is compatible with target browsers:
update the expressions that set currentSorted and originalSorted (currently
using current.toSorted() and original.toSorted()) to use [...current].sort() (or
Array.from(current).sort()) instead, preserving the original sort comparator if
present.
In @.claude/skills/react-best-practices/references/rules/rendering-hoist-jsx.md:
- Around line 14-42: The examples reference an undefined loading variable and
mix two different patterns; update both examples so they compile and clearly
demonstrate the hoisting pattern: change Container to accept a loading prop
(function Container({ loading }: { loading: boolean })) or otherwise define
loading as state, replace the incorrect example's usage to either call the
reusable component (<LoadingSkeleton />) or use the hoisted element
(loadingSkeleton) consistently, ensure the hoisted constant loadingSkeleton is
declared outside or inside Container as shown, and add a brief note that
hoisting is only beneficial for truly static JSX that never depends on
props/state and is reused conditionally.
In
@.claude/skills/react-best-practices/references/rules/rerender-defer-reads.md:
- Around line 27-38: Add caveats and a "When to use" section to the ShareButton
example: explain that reading window.location.search inside handleShare is a
read-on-demand pattern that won’t trigger re-renders if the URL changes, is
invalid during SSR/Server Components because window is undefined, and differs
from Next.js’s useSearchParams() which provides reactive updates; state that
this pattern is appropriate only for client components and one-off event reads
(like ShareButton.handleShare) or non-Next.js contexts, and recommend
useSearchParams() for URL-driven reactivity in Next.js App Router scenarios.
In
@.claude/skills/react-best-practices/references/rules/rerender-transitions.md:
- Around line 12-23: Update the wording and example in the ScrollTracker snippet
to clarify that frequent scroll updates are treated as urgent by React (not
literally "blocking" the UI) and show the rendering impact: replace the phrase
"blocks UI on every scroll" with wording about "updates are treated as urgent by
default; use startTransition to make them non‑urgent/interruptible", and
complete the example by returning JSX that uses scrollY (e.g., render
<ExpensiveComponent scrollY={scrollY} />) and include a second snippet
demonstrating how to wrap setScrollY in React.startTransition to mark updates as
non‑urgent; reference the ScrollTracker component, setScrollY, startTransition,
and ExpensiveComponent so readers can find and test the behavior.
- Around line 25-40: Extend the "Correct (non-blocking updates)" ScrollTracker
example to include rendering logic that shows the impact of using
startTransition: after the useEffect block, return a JSX fragment that renders
an ExpensiveComponent receiving scrollY (e.g., <ExpensiveComponent
scrollY={scrollY} />) and a short comment explaining that scroll updates are
non-urgent; keep the same state and handler (scrollY, setScrollY,
startTransition) so readers can see how startTransition prevents scroll updates
from blocking the expensive render.
In @.claude/skills/react-best-practices/references/rules/server-cache-react.md:
- Around line 8-10: Update the guidance under the "Per-Request Deduplication
with React.cache()" section to explicitly state that React.cache() only works in
React Server Components (RSC) and will not run in client-side code or browser
bundles; keep existing availability notes intact. Locate the paragraph
mentioning React.cache() and append or replace with a concise sentence like
"Note: React.cache() is only supported in React Server Components and has no
effect in client-side code." to make the RSC requirement unambiguous while
leaving the rest of the advice unchanged.
In
@.claude/skills/react-best-practices/references/rules/server-parallel-fetching.md:
- Around line 1-79: This reference on Parallel Data Fetching incorrectly targets
RSC/Next.js server-side patterns that don't apply to our client-only
shapeshift/web; update the file by either removing it or adding a prominent
applicability note at the top clarifying it's for RSC/App Router only and not
for shapeshift/web, and mention that constructs referenced (async Server
Components like Page, Header, Sidebar, Layout and server data fetchers like
fetchHeader/fetchSidebarItems or utilities such as React.cache()) are not
supported in this repo so readers should ignore or adapt those examples for
client-side equivalents.
In @.claude/skills/react-best-practices/SKILL.md:
- Around line 59-64: Replace the misleading per-icon import example in SKILL.md
(the line showing import Check from 'lucide-react/dist/esm/icons/check') with
the official recommended named-import pattern: use the named export Check via
import { Check } from 'lucide-react' and update the surrounding comment to state
that modern bundlers tree-shake named imports by default; remove or rephrase the
per-icon path suggestion and any claim that it’s the preferred approach without
extra bundler config.
In `@src/pages/Yields/components/YieldsList.tsx`:
- Around line 497-506: The comparator in filtered.sort uses aBalance and
bBalance which may be undefined due to optional chaining on allBalances; ensure
monotonic numeric results by null-coalescing both results to a zero BigNumber
before calling .minus() and .toNumber() (e.g., use aBalance ?? bnOrZero(0) and
bBalance ?? bnOrZero(0)); update the comparator in the YieldsList component so
allBalances[a.id]?.reduce(...) and allBalances[b.id]?.reduce(...) are wrapped
with ?? bnOrZero(0) before invoking .minus() to prevent runtime errors or NaN.
🧹 Nitpick comments (33)
.claude/skills/react-best-practices/references/rules/rerender-dependencies.md (1)
28-45: Consider adding useMemo to align with team's memoization practices.The derived state example is technically correct and pedagogically clear. However, the team's documented practice (from learnings and
.cursor/rules/react-best-practices.mdc) is to useuseMemofor "all derived values and conditional values."Based on learnings, the aggressive memoization pattern would be:
const isMobile = useMemo(() => width < 768, [width]) useEffect(() => { if (isMobile) { enableMobileMode() } }, [isMobile])That said, the current example prioritizes teaching the narrow-dependency concept without the noise of memoization hooks, which may be preferable for documentation clarity.
Based on learnings, the ShapeShift team practices aggressive memoization for all derived values.
.claude/skills/react-best-practices/references/rules/js-hoist-regexp.md (1)
22-35: Clarify the example by removing unused constant or adding escapeRegex definition.The "correct" example has two clarity issues:
EMAIL_REGEXis defined on line 25 but never used in theHighlightercomponent. If it's meant to demonstrate module-level hoisting (per line 10's guidance), consider showing it in a separate example or removing it.
escapeRegex(query)on line 29 is called but never defined, which may confuse readers trying to understand the complete pattern.📚 Suggested improvements for clarity
Option 1: Remove the unused EMAIL_REGEX constant:
-const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ - function Highlighter({ text, query }: Props) { const regex = useMemo( - () => new RegExp(`(${escapeRegex(query)})`, 'gi'), + () => new RegExp(`(${query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi'), [query] )Option 2: Use EMAIL_REGEX in a separate example demonstrating module-level hoisting:
+// Module-level hoisting for static patterns const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ +function EmailValidator({ email }: Props) { + const isValid = EMAIL_REGEX.test(email) + return <div>{isValid ? '✓' : '✗'}</div> +} + +// useMemo for dynamic patterns function Highlighter({ text, query }: Props) { const regex = useMemo( - () => new RegExp(`(${escapeRegex(query)})`, 'gi'), + () => new RegExp(`(${query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')})`, 'gi'), [query] ).claude/skills/react-best-practices/references/rules/js-min-max-loop.md (1)
74-82: Consider clarifying the spread operator limitation.The caveat about spread operators is valid, but could be more specific. The issue isn't just performance—with extremely large arrays (typically 65k-125k+ elements depending on the JS engine),
Math.max(...array)can throw aRangeError: Maximum call stack size exceededbecause the spread operator passes each element as a separate function argument.📝 Optional clarification
-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 fail with a `RangeError: Maximum call stack size exceeded` for extremely large arrays (typically 65k-125k+ elements) because the spread operator passes each element as a separate function argument. Use the loop approach for reliability with arbitrarily large arrays..claude/skills/react-best-practices/references/rules/js-combine-iterations.md (1)
8-10: Add guidance on when to apply this optimization.The rule presents the imperative loop as universally "correct" without discussing trade-offs. For small-to-medium arrays (most use cases), the
.filter()approach is more readable and maintainable, and the performance difference is negligible. This optimization should be applied judiciously—primarily for large arrays in hot paths identified through profiling.Consider adding a note like: "Apply this optimization when profiling shows array iteration is a bottleneck. For most cases, prefer the more readable
.filter()approach."📝 Suggested addition to provide context
## Combine Multiple Array Iterations Multiple `.filter()` or `.map()` calls iterate the array multiple times. Combine into one loop. + +**When to apply:** Use this optimization for large arrays (1000+ items) in performance-critical code paths. For typical use cases, prefer the more readable `.filter()` approach unless profiling identifies a bottleneck. **Incorrect (3 iterations):**.claude/skills/react-best-practices/references/rules/bundle-defer-third-party.md (1)
4-4: Consider more precise terminology for the deferral behavior.The description "loads after hydration" is slightly imprecise. With
{ ssr: false }, the component is client-side only and the JavaScript is code-split/loaded asynchronously, but it renders during the client-side render phase (part of hydration) rather than strictly "after" hydration completes.Consider alternatives like "client-side only", "code-split and loaded asynchronously", or "doesn't block initial bundle" for technical precision.
Also applies to: 10-10
.claude/skills/react-best-practices/references/rules/client-event-listeners.md (2)
38-55: Document callback stability requirement.Since
callbackis in the dependency array (line 55), consumers must wrap their callbacks inuseCallbackto avoid constant re-registration. Without this, theuseEffectwill run on every render, repeatedly adding/removing the same callback from the Map.Consider adding a note to the documentation or updating the example to demonstrate this requirement:
📝 Example showing useCallback requirement
function Profile() { const handleP = useCallback(() => { console.log('P pressed') }, []) const handleK = useCallback(() => { console.log('K pressed') }, []) useKeyboardShortcut('p', handleP) useKeyboardShortcut('k', handleK) }Or add a documentation note:
**Note:** Callbacks passed to `useKeyboardShortcut` should be wrapped in `useCallback` to prevent unnecessary re-registration.
68-74: Consider showing same-key usage across multiple components to better demonstrate deduplication.The current example shows different keys ('p' and 'k') sharing the global listener, which is correct. However, showing the same key used across multiple component instances would more clearly demonstrate the core deduplication benefit (N instances registering the same shortcut = 1 event listener + N callbacks in the Map).
💡 Alternative example showing same-key deduplication
function SaveButton() { useKeyboardShortcut('s', () => console.log('Save from button')) return <button>Save</button> } function SaveMenuItem() { useKeyboardShortcut('s', () => console.log('Save from menu')) return <MenuItem>Save</MenuItem> } function Editor() { return ( <> <SaveButton /> <SaveMenuItem /> {/* Both components register 's', but only ONE global keydown listener exists */} </> ) }.claude/skills/react-best-practices/references/rules/server-cache-react.md (1)
14-24: Clarify Server Components context in the example.The example demonstrates a valid pattern but should explicitly state it's for Server Components. Consider adding a comment or note clarifying where this code should live (e.g., "This function must be defined in a Server Component file or server-side utility module").
📝 Suggested enhancement
**Usage:** ```typescript +// In a Server Component or server-side utility import { cache } from 'react' export const getCurrentUser = cache(async () => {.claude/skills/react-best-practices/references/rules/server-cache-lru.md (2)
14-33: Improve type safety and code completeness in the example.The implementation has two issues that reduce its value as a reference:
- Line 17: Using
anydefeats TypeScript's type safety. Consider using a generic type parameter or at leastunknown.- Line 26: The
dbobject is undefined. Add an import statement or a comment explaining it's a placeholder.Based on learnings, gomesalexandre values TypeScript-first approaches and proper type definitions.
📝 Proposed improvements
**Implementation:** ```typescript import { LRUCache } from 'lru-cache' +import { db } from './db' // or: const db = prisma + +type User = { + id: string + name: string + // ... other fields +} -const cache = new LRUCache<string, any>({ +const cache = new LRUCache<string, User>({ max: 1000, ttl: 5 * 60 * 1000 // 5 minutes }) export async function getUser(id: string) { const cached = cache.get(id) if (cached) return cached const user = await db.user.findUnique({ where: { id } }) cache.set(id, user) return user }Alternatively, if keeping it generic:
-const cache = new LRUCache<string, any>({ +// Replace 'any' with your specific type (e.g., User, Product, etc.) +const cache = new LRUCache<string, unknown>({ max: 1000, ttl: 5 * 60 * 1000 // 5 minutes })
35-35: Consider adding cache invalidation guidance.The usage guidance is clear, but documentation could benefit from mentioning cache invalidation strategies (e.g., when user data is updated, how to invalidate the cache) and considerations for stale data with the 5-minute TTL.
Example addition:
Use when sequential user actions hit multiple endpoints needing the same data within seconds. In serverless, consider Redis for cross-process caching. **Important considerations:** - Implement cache invalidation when data is mutated (e.g., `cache.delete(id)` after user updates) - Be aware of stale data within the TTL window - Monitor memory usage in long-running processessrc/components/Layout/Header/Header.tsx (1)
86-95: Hoist static display objects to module scope.These objects are static; keeping them inside the component recreates them every render. Consider hoisting them alongside the other style constants.
♻️ Suggested refactor
const searchBoxMaxWSx = { base: 'auto', lg: '400px' } const searchBoxMinWSx = { base: 'auto', xl: '300px' } +const searchBoxDisplay = { base: 'none', '2xl': 'flex', xl: 'none' } +const iconButtonDisplay = { base: 'flex', '2xl': 'none' } @@ - const searchBoxDisplay = { - base: 'none', - '2xl': 'flex', - xl: 'none', - } - - const iconButtonDisplay = { - base: 'flex', - '2xl': 'none', - }As per coding guidelines, prefer hoisting static style objects to avoid per-render allocations.
.claude/skills/react-best-practices/references/rules/js-cache-property-access.md (1)
10-10: Consider clarifying "hot paths" terminology.The term "hot paths" is developer jargon that may not be immediately clear to all readers. Consider briefly defining it (e.g., "frequently executed code paths") or providing more context.
📝 Suggested clarification
-Cache object property lookups in hot paths. +Cache object property lookups in hot paths (frequently executed code, such as tight loops)..claude/skills/react-best-practices/references/rules/async-parallel.md (1)
8-28: Document error handling trade-offs for Promise.all().The guidance correctly shows parallelization benefits, but omits a critical behavioral difference: with
Promise.all(), if any promise rejects, the entire operation fails immediately. Sequential awaits fail at the first error but don't attempt subsequent calls.Consider adding:
- A note about error handling behavior
- Recommendation to use
Promise.allSettled()when partial results are acceptable- Emphasis that operations must be truly independent (no data dependencies)
📚 Suggested addition
**Error Handling Considerations:** If any operation can fail independently and you need partial results, use `Promise.allSettled()`: ```typescript const results = await Promise.allSettled([ fetchUser(), fetchPosts(), fetchComments() ]) // Handle each result results.forEach((result, index) => { if (result.status === 'fulfilled') { // Use result.value } else { // Handle result.reason } })Important: Only use
Promise.all()when operations are truly independent with no data dependencies between them.</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/rendering-svg-precision.md (1)</summary><blockquote> `8-28`: **Clarify precision trade-offs and provide selection guidance.** While the document mentions "optimal precision depends on the viewBox size," it doesn't explain how to determine the appropriate precision level. Using `--precision=1` universally may be too aggressive for certain use cases: - Icons scaled to various sizes may show visual artifacts with precision=1 - Larger viewBox sizes can tolerate lower precision better than smaller ones - Complex paths benefit from higher precision to maintain curve quality Consider adding guidance on: 1. How to determine appropriate precision for a given viewBox 2. Visual testing recommendations before/after optimization 3. A more conservative default (e.g., precision=2 or 3) with a note to reduce further if file size is critical <details> <summary>📝 Suggested enhancement</summary> ```markdown **Choosing the Right Precision:** A good starting point is `precision=2` for most icons. Reduce to `precision=1` only after visual inspection confirms no quality loss. For large or complex SVGs, `precision=3` may be necessary. Rule of thumb: `precision = Math.ceil(Math.log10(viewBox_size))` Always visually compare the optimized SVG at various scales before committing..claude/skills/react-best-practices/references/rules/js-set-map-lookups.md (1)
8-24: Add context about when this optimization is beneficial.While the O(1) vs. O(n) complexity is correct, the document doesn't mention when this optimization actually provides measurable benefits. Creating a
Sethas overhead that may not be worthwhile for small arrays:
- For arrays with < ~10-20 items,
Array.includes()may be faster due to Set creation cost- Benefit is most significant when:
- The array is large (100+ items)
- Multiple lookups are performed against the same collection
- Lookups occur in performance-critical paths
- If converting for a single lookup, the Set creation overhead may negate the benefit
Consider adding a note about when to apply this optimization, or adjust the impact level to reflect that it's only beneficial for specific scenarios.
📝 Suggested addition
**When to Apply:** Use Set/Map when: - The collection has 20+ items - Multiple lookups will be performed - The lookup is in a hot path (e.g., inside `filter`, `map`, or a loop) For small collections or one-time lookups, the Set creation overhead may exceed the benefit..claude/skills/react-best-practices/references/rules/download_rules.sh (1)
1-63: Consider adding error handling for robustness.The script will print "Downloaded all rule files successfully!" even if some downloads fail silently due to
-sflag. For a developer utility script, this may be acceptable, but consider adding basic error handling.🔧 Optional: Add fail-fast behavior and download verification
#!/bin/bash +set -euo pipefail + BASE_URL="https://raw.githubusercontent.com/vercel-labs/agent-skills/react-best-practices/skills/react-best-practices/references/rules" +download_file() { + local file="$1" + if ! curl -sf "$BASE_URL/$file" -o "$file"; then + echo "Failed to download: $file" >&2 + return 1 + fi +} + # Async rules -curl -s "$BASE_URL/async-api-routes.md" > async-api-routes.md +download_file "async-api-routes.md" # ... (apply to all files).claude/skills/react-best-practices/references/rules/advanced-event-handler-refs.md (1)
1-38: DocumentuseEffectEventas the preferred modern solution for React 19.2+.React 19.2 introduced
useEffectEvent()specifically to provide stable event handler identity without triggering effect re-subscriptions. The manual ref pattern shown is technically correct but essentially reimplements whatuseEffectEventprovides natively. Update the document to recommenduseEffectEventas the primary approach while noting the ref pattern for earlier React versions.📝 Suggested update
## Store Event Handlers in Refs Store callbacks in refs when used in effects that shouldn't re-subscribe on callback changes. + +**React 19.2+:** Use `useEffectEvent()` for this pattern—it provides stable event handler identity without the manual ref bookkeeping. **Incorrect (re-subscribes on every render):**Or add a separate section documenting
useEffectEventas the recommended modern approach..claude/skills/react-best-practices/references/rules/bundle-barrel-imports.md (2)
40-53: Update Next.js feature status.The
optimizePackageImportsfeature graduated from experimental status in Next.js 13.5+ and is stable in Next.js 14+. The documentation should reflect the current stable status rather than labeling it as experimental.📝 Suggested update
-**Alternative (Next.js 13.5+):** +**Alternative (Next.js 13.5+, stable in 14+):** ```js -// next.config.js - use optimizePackageImports +// next.config.js module.exports = { - experimental: { - optimizePackageImports: ['lucide-react', '@mui/material'] - } + optimizePackageImports: ['lucide-react', '@mui/material'] }</details> --- `16-38`: **Add version-specific caveat for import paths.** Direct import paths vary across library versions. For example, `lucide-react` changed its structure in v0.263.0+ (now uses `/icons/` instead of `/dist/esm/icons/`). Consider adding a note advising readers to verify import paths against their installed library version. <details> <summary>📝 Suggested addition</summary> Add after line 38: ```markdown **Note:** Import paths vary by library version. Always verify the correct path structure for your installed version by checking the library's package.json exports or documentation..claude/skills/react-best-practices/references/rules/js-cache-storage.md (2)
21-37: Add deletion helper to maintain cache consistency.The example provides
getLocalStorageandsetLocalStoragebut omitsdeleteLocalStorage. Deletion operations will cause cache staleness if the cache isn't invalidated.➕ Add deletion helper
function deleteLocalStorage(key: string) { localStorage.removeItem(key) storageCache.delete(key) // keep cache in sync }
56-67: Consider adding memory management guidance.The cache invalidation strategy clears the entire cache on visibility change, which is good. However, the document doesn't mention that caches can grow unbounded in long-lived applications. Consider adding a note about potential memory implications and when to implement size limits (e.g., LRU eviction) for applications with many storage keys.
.claude/skills/react-best-practices/references/rules/async-api-routes.md (1)
38-38: Add context or link for "better-all" reference.The document references
better-allfor complex dependency chains but provides no explanation, import path, or link. Readers won't know what this library is or how to use it.📚 Suggested addition
For operations with more complex dependency chains, use [`better-all`](https://www.npmjs.com/package/better-all) to automatically maximize parallelism (see Dependency-Based Parallelization): ```typescript import { all } from 'better-all' const result = await all({ auth: auth(), config: fetchConfig(), data: async ({ auth }) => fetchData(auth.user.id) })</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/async-suspense-boundaries.md (1)</summary><blockquote> `8-66`: **LGTM - Excellent Suspense boundary guidance.** The document provides clear, accurate guidance on strategic Suspense boundaries with well-chosen examples. The "When NOT to use" section is particularly valuable as it helps readers make informed trade-offs between faster initial paint and layout stability. One optional enhancement: Consider mentioning error boundaries alongside Suspense boundaries, as they often work together (Suspense handles loading states, error boundaries handle error states in streaming scenarios). </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/js-cache-function-results.md (1)</summary><blockquote> `31-42`: **Consider noting cache eviction for unbounded inputs.** The module-level cache pattern is correct. For completeness, consider adding a note about cache eviction strategies (e.g., LRU) when the input domain is unbounded, to prevent memory growth in long-running applications. The current example works well for bounded sets like project names. </blockquote></details> <details> <summary>src/pages/Yields/components/YieldOpportunityStats.tsx (1)</summary><blockquote> `15-15`: **Re-memoize the derived yield aggregates.** The active/idle value and APY aggregations now recompute on every render; the reductions are non-trivial and should stay memoized per project performance guidance. <details> <summary>♻️ Suggested memoization</summary> ```diff -import { memo } from 'react' +import { memo, useMemo } from 'react' @@ -const activeValueUsd = positions.reduce((acc, position) => { - const positionBalances = balances?.[position.id] - if (!positionBalances) return acc - const activeBalances = positionBalances.filter(b => b.type === 'active' || b.type === 'locked') - return activeBalances.reduce((sum, b) => sum.plus(bnOrZero(b.amountUsd)), acc) -}, bnOrZero(0)) +const activeValueUsd = useMemo( + () => + positions.reduce((acc, position) => { + const positionBalances = balances?.[position.id] + if (!positionBalances) return acc + const activeBalances = positionBalances.filter( + b => b.type === 'active' || b.type === 'locked', + ) + return activeBalances.reduce((sum, b) => sum.plus(bnOrZero(b.amountUsd)), acc) + }, bnOrZero(0)), + [positions, balances], +) @@ -const idleValueUsd = (() => { +const idleValueUsd = useMemo(() => { if (!isConnected || !allYields) return bnOrZero(0) @@ -})() +}, [isConnected, allYields, portfolioBalances]) @@ -const { weightedApy, potentialEarningsValue } = (() => { +const { weightedApy, potentialEarningsValue } = useMemo(() => { if (!isConnected || !yields?.byInputAssetId || !portfolioBalances) { return { weightedApy: 0, potentialEarningsValue: bnOrZero(0) } } @@ -})() +}, [isConnected, yields?.byInputAssetId, portfolioBalances])As per coding guidelines, ...
Also applies to: 56-105
src/pages/Yields/components/YieldRelatedMarkets.tsx (1)
15-19: Consider adding a displayName for debugging.The component uses an anonymous function with
memo(). While functionally correct, adding a named function ordisplayNameimproves React DevTools debugging:-export const YieldRelatedMarkets = memo( - ({ currentYieldId, tokenSymbol }: YieldRelatedMarketsProps) => { +export const YieldRelatedMarkets = memo(function YieldRelatedMarkets({ + currentYieldId, + tokenSymbol, +}: YieldRelatedMarketsProps) {src/pages/Yields/components/YieldInfoCard.tsx (3)
40-48: Consider adding displayName and memoizing computed values.Per coding guidelines, computed values should use
useMemo. The component also uses an anonymous function withmemo():♻️ Suggested improvements
-export const YieldInfoCard = memo( - ({ yieldItem, validatorOrProvider, titleOverride }: YieldInfoCardProps) => { +export const YieldInfoCard = memo(function YieldInfoCard({ + yieldItem, + validatorOrProvider, + titleOverride, +}: YieldInfoCardProps) { const translate = useTranslate() - const iconSource = resolveYieldInputAssetIcon(yieldItem) - const apy = bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2) - const yieldTitle = titleOverride ?? getYieldDisplayName(yieldItem) - const type = yieldItem.mechanics.type - const description = yieldItem.metadata.description + const iconSource = useMemo(() => resolveYieldInputAssetIcon(yieldItem), [yieldItem]) + const apy = useMemo( + () => bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2), + [yieldItem.rewardRate.total], + ) + const yieldTitle = useMemo( + () => titleOverride ?? getYieldDisplayName(yieldItem), + [titleOverride, yieldItem], + ) + const type = yieldItem.mechanics.type + const description = yieldItem.metadata.description
50-87: Consider memoizing the icon elements.The
assetIconandstackedIconElementcomputations include JSX creation and conditional logic. Per coding guidelines, these could benefit fromuseMemo:♻️ Suggested improvement
- const assetIcon = iconSource.assetId ? ( + const assetIcon = useMemo(() => iconSource.assetId ? ( <AssetIcon assetId={iconSource.assetId} size='lg' /> ) : ( <AssetIcon src={iconSource.src} size='lg' /> - ) + ), [iconSource.assetId, iconSource.src]) - const hasOverlay = validatorOrProvider?.logoURI || yieldItem.chainId + const hasOverlay = validatorOrProvider?.logoURI || yieldItem.chainId - const stackedIconElement = !hasOverlay ? ( + const stackedIconElement = useMemo(() => !hasOverlay ? ( assetIcon ) : ( // ... overlay logic - ) + ), [hasOverlay, assetIcon, validatorOrProvider, yieldItem.chainId])
145-178: Asset icon rendering is duplicated.Lines 147-151 duplicate the same conditional
AssetIconpattern from lines 50-54. Consider extracting a shared element:+ const assetIconXs = useMemo( + () => + iconSource.assetId ? ( + <AssetIcon assetId={iconSource.assetId} size='xs' /> + ) : ( + <AssetIcon src={iconSource.src} size='xs' /> + ), + [iconSource.assetId, iconSource.src], + )Then use
{assetIconXs}in the HStack. This reduces duplication and follows the guideline to memoize JSX elements.src/pages/Yields/components/YieldProviderInfo.tsx (2)
16-24: Consider using a named function for memo.Same pattern as other components - adding a named function improves debugging:
-export const YieldProviderInfo = memo( - ({ providerId, providerName, providerLogoURI, providerWebsite }: YieldProviderInfoProps) => { +export const YieldProviderInfo = memo(function YieldProviderInfo({ + providerId, + providerName, + providerLogoURI, + providerWebsite, +}: YieldProviderInfoProps) {The
useMemofor description lookup is appropriate.
43-55: Button inside Link creates nested interactive elements.Wrapping a
Buttoninside aLinkcreates nested interactive elements which can cause accessibility issues (focusable elements within focusable elements). Consider usingLinkwith button styling orButtonwithas={Link}:♻️ Alternative approach
- {providerWebsite && ( - <Link href={providerWebsite} isExternal ml={-2}> - <Button - variant='ghost' - size='sm' - rightIcon={<ExternalLinkIcon />} - color='text.subtle' - _hover={{ color: 'text.base' }} - > - {translate('yieldXYZ.visitWebsite')} - </Button> - </Link> - )} + {providerWebsite && ( + <Button + as={Link} + href={providerWebsite} + isExternal + variant='ghost' + size='sm' + rightIcon={<ExternalLinkIcon />} + color='text.subtle' + _hover={{ color: 'text.base', textDecoration: 'none' }} + ml={-2} + > + {translate('yieldXYZ.visitWebsite')} + </Button> + )}Based on learnings, this can be deferred to a follow-up if preferred.
src/pages/Yields/components/YieldItem.tsx (1)
78-110: Consider memoizing thestatscomputation for group yields.The stats IIFE performs array operations (
map,reduce,Set) on potentially largeyieldsarrays on every render. While this may be acceptable for small datasets, it could impact performance for groups with many yields.♻️ Optional: Memoize stats computation
- const stats = (() => { + const stats = useMemo(() => { if (isSingle) { const y = data.yieldItem return { apy: y.rewardRate.total, apyLabel: y.rewardRate.rateType, tvlUsd: y.statistics?.tvlUsd ?? '0', providers: [{ id: y.providerId, logo: data.providerIcon }], chainIds: y.chainId ? [y.chainId] : [], count: 1, name: y.metadata.name, canEnter: y.status.enter, } } const yields = data.yields const maxApy = Math.max(0, ...yields.map(y => y.rewardRate.total)) const totalTvlUsd = yields .reduce((acc, y) => acc.plus(bnOrZero(y.statistics?.tvlUsd)), bnOrZero(0)) .toFixed() const providerIds = [...new Set(yields.map(y => y.providerId))] const chainIds = [...new Set(yields.map(y => y.chainId).filter(Boolean))] as string[] return { apy: maxApy, apyLabel: 'APY', tvlUsd: totalTvlUsd, providers: providerIds.map(id => ({ id, logo: yieldProviders?.[id]?.logoURI })), chainIds, count: yields.length, name: data.assetName, canEnter: true, } - })() + }, [data, isSingle, yieldProviders])src/pages/Yields/YieldDetail.tsx (1)
157-185: Consider memoizinguserBalancescomputation.This IIFE performs multiple
reduceoperations on arrays. While the dependencies are limited, this computation runs on every render. Given thatbalances,selectedValidatorAddress, anduserCurrencyToUsdRateare likely to change together with other state, this may cause unnecessary recalculations.♻️ Optional: Wrap in useMemo
- const userBalances = (() => { + const userBalances = useMemo(() => { if (!balances) return { userCurrency: '0', crypto: '0' } // ... rest of computation return { userCurrency: totalUsd.times(userCurrencyToUsdRate).toFixed(), crypto: totalCrypto.toFixed(), } - })() + }, [balances, selectedValidatorAddress, userCurrencyToUsdRate])
.claude/skills/react-best-practices/references/rules/bundle-conditional.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/client-event-listeners.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/js-cache-property-access.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/js-cache-property-access.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/js-cache-storage.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/rerender-transitions.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/server-cache-react.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/server-parallel-fetching.md
Show resolved
Hide resolved
- Fix dangerous fallback to 18 decimals in YieldAvailableToDeposit - Remove unnecessary Box wrapper in YieldEnterModal - Simplify nested ternary in YieldDetail using useMemo - Refactor YieldExplainers with memoized translations - Convert function keywords to arrow functions in memo components - Use implicit returns in useMemo/useCallback - Simplify enum mappings in YieldsList using array pattern - Remove NEAR_IMPROVEMENTS.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Yields/YieldDetail.tsx (1)
103-121: Default validator selection skips non-mapped validator yields.
defaultValidatoronly usesDEFAULT_NATIVE_VALIDATOR_BY_CHAIN_ID. For staking yields that require validator selection but aren’t in that mapping (and lack avalidatorparam),selectedValidatorAddressbecomes undefined, which can leave validator info/balances inconsistent withYieldStats(which falls back tovalidators?.[0]). Consider adding that fallback and moving the selection below the validator fetch.🐛 Suggested fix
- const defaultValidator = yieldItem?.chainId - ? DEFAULT_NATIVE_VALIDATOR_BY_CHAIN_ID[yieldItem.chainId] - : undefined - - const selectedValidatorAddress = (() => { - if ( - yieldId === COSMOS_ATOM_NATIVE_STAKING_YIELD_ID || - yieldId === SOLANA_SOL_NATIVE_MULTIVALIDATOR_STAKING_YIELD_ID || - (yieldId?.includes('solana') && yieldId?.includes('native')) - ) { - return defaultValidator - } - return validatorParam || defaultValidator - })() - - const isStaking = yieldItem?.mechanics.type === 'staking' - const shouldFetchValidators = isStaking && yieldItem?.mechanics.requiresValidatorSelection - const { data: validators } = useYieldValidators(yieldItem?.id ?? '', shouldFetchValidators) + const isStaking = yieldItem?.mechanics.type === 'staking' + const shouldFetchValidators = isStaking && yieldItem?.mechanics.requiresValidatorSelection + const { data: validators } = useYieldValidators(yieldItem?.id ?? '', shouldFetchValidators) + + const defaultValidator = + (yieldItem?.chainId && DEFAULT_NATIVE_VALIDATOR_BY_CHAIN_ID[yieldItem.chainId]) ?? + validators?.[0]?.address + + const selectedValidatorAddress = (() => { + if ( + yieldId === COSMOS_ATOM_NATIVE_STAKING_YIELD_ID || + yieldId === SOLANA_SOL_NATIVE_MULTIVALIDATOR_STAKING_YIELD_ID || + (yieldId?.includes('solana') && yieldId?.includes('native')) + ) { + return defaultValidator + } + return validatorParam || defaultValidator + })()
🤖 Fix all issues with AI agents
In
@.claude/skills/react-best-practices/references/react-performance-guidelines.md:
- Around line 18-115: The TOC anchors in "Table of Contents" (e.g.,
"#1-eliminating-waterfalls", "#11") do not match the generated heading slugs and
bolded lines like "**Impact: CRITICAL**" are being used as pseudo-headings,
triggering MD051 and MD036; update the TOC entries to match actual heading IDs
(or add explicit anchor tags immediately above headings such as <a
name="1-eliminating-waterfalls"></a>) and replace bold pseudo-headings under
sections like "## 1. Eliminating Waterfalls" and "### 1.1 Defer Await Until
Needed" with real heading levels (e.g., "#### Impact: CRITICAL" or "####
Incorrect:" / "#### Correct:") or add a markdownlint disable directive for MD036
at the top of the file if you intentionally want to keep the bold styling;
ensure the anchors and heading text exactly match (case and punctuation) so
MD051 is resolved.
In @.claude/skills/react-best-practices/references/rules/bundle-conditional.md:
- Around line 15-24: The example in AnimationPlayer uses an undefined
setEnabled; update the component signature to accept an onDisable callback prop
(e.g., function onDisable(): void) and call onDisable() inside the import .catch
handler instead of setEnabled(false), and include onDisable in the useEffect
dependency array so the effect is stable; reference the AnimationPlayer
function, the useEffect block, and the .catch handler when making this change.
In
@.claude/skills/react-best-practices/references/rules/bundle-defer-third-party.md:
- Around line 32-37: Replace the Next.js-specific dynamic import pattern using
dynamic(...) and the Analytics symbol with React's lazy + Suspense pattern:
change the import so Analytics is created via React.lazy(() =>
import('@vercel/analytics/react').then(m => ({ default: m.Analytics }))) and
render it wrapped in a Suspense fallback when used; optionally use the project's
makeSuspenseful utility (makeSuspenseful in src/utils/makeSuspenseful.tsx)
instead of raw Suspense for consistency. Ensure you remove dynamic(...) usage
and update all render sites to import Suspense and render <Analytics /> inside
the fallback wrapper.
In @.claude/skills/react-best-practices/references/rules/bundle-preload.md:
- Line 50: Update the sentence that claims "The `typeof window !== 'undefined'`
check prevents bundling preloaded modules for SSR" to clarify runtime vs
build-time: explain that the `typeof window !== 'undefined'` guard prevents
server-side execution at runtime but does not stop the module from being
included in the overall bundle; instead, modern bundlers will emit it as a
client-only chunk that bypasses server-side loading, which reduces server bundle
size and can speed builds. Keep reference to the exact symbol `typeof window !==
'undefined'` and reword the paragraph to state that it prevents server execution
while resulting in a client chunk rather than implying it prevents bundling.
In
@.claude/skills/react-best-practices/references/rules/client-event-listeners.md:
- Around line 57-66: The useSWRSubscription call is missing a closing
parenthesis; add the missing ')' after the subscription callback so the
invocation of useSWRSubscription('global-keydown', () => { ... }) is properly
closed; locate the useSWRSubscription(...) block that defines handler (the
KeyboardEvent listener referencing keyCallbacks and keyCallbacks.get) and ensure
the returned cleanup arrow function and the outer callback are closed and then
append the final ')' to close the useSWRSubscription call.
- Around line 38-55: The documentation's useKeyboardShortcut example and text
must state that the callback passed to useKeyboardShortcut needs to be memoized
to avoid effect thrashing: update the docs around the useKeyboardShortcut
function and the usage example to either wrap event handlers in React's
useCallback (e.g., show Profile using handleP/useCallback and
handleK/useCallback) or add a clear note immediately after the
useKeyboardShortcut code block explaining "callbacks passed to
useKeyboardShortcut must be memoized (e.g., via useCallback) to prevent constant
registration/unregistration." Ensure you reference the useKeyboardShortcut
function and the callback dependency when making the change.
In @.claude/skills/react-best-practices/references/rules/client-swr-dedup.md:
- Around line 45-54: The example uses useSWRMutation('/api/user', updateUser)
but never defines updateUser; add a concise description of the expected mutation
function signature and behavior (what params it receives and that it should
perform the API call and return the result) and include a short example
implementation for updateUser referenced by useSWRMutation so readers can see
the proper pattern for mutations used by trigger. Reference the updateUser
function, useSWRMutation hook, and trigger callback in the explanation.
- Around line 35-43: Add documentation above the snippet clarifying what
useImmutableSWR does (e.g., how it differs from useSWR, that it disables
revalidation/refresh for truly immutable data), when to use it versus standard
useSWR, and what "immutable" means in this context (data that never changes
after initial fetch); also ensure fetcher is defined or imported and mention its
expected shape (a function that takes a URL and returns parsed data). Update the
example block to include a one-line comment like "useImmutableSWR disables
revalidation for data that never changes" and either import or reference a
defined fetcher so readers know how to supply it.
- Around line 27-33: The example uses useSWR('/api/users', fetcher) but never
defines or explains fetcher; add a short definition or explanatory note for
fetcher used by useSWR (e.g., a function that accepts a URL and returns parsed
JSON) and update the UserList example to reference that defined fetcher so
readers know what to pass as the second argument to useSWR (mention the symbol
fetcher and the component UserList in the change).
In @.claude/skills/react-best-practices/references/rules/download_rules.sh:
- Around line 1-63: The script download_rules.sh currently uses silent curl (-s)
for every download and always prints "Downloaded all rule files successfully!"
even on failures; update the script to fail fast by enabling strict mode (set
-euo pipefail), replace curl -s with curl --fail --silent --show-error and add
retries (e.g., --retry and --retry-connrefused) for each download command that
references BASE_URL/*, and ensure each curl failure stops the script before the
final echo (or check exit status of each curl and exit non-zero) so the success
message is only printed when all downloads actually succeed.
In
@.claude/skills/react-best-practices/references/rules/js-combine-iterations.md:
- Around line 8-18: Update the guidance in the "Combine Multiple Array
Iterations" rule to reframe it as a context-dependent optimization rather than
labeling `.filter()` usage as outright "Incorrect": change the "Incorrect"
heading to "Readable but potentially inefficient" and change the "Correct"
heading to "Optimized for large arrays", add a short note recommending the
mutation/single-loop approach only when profiling shows a bottleneck (e.g., >10k
items or hot render paths), explicitly call out the React tradeoff around
referential equality and memoization (mention dependency arrays/memo/useEffect),
and advise preferring TypeScript-first, declarative `.filter()` for readability
and avoiding `let` assignments unless proven necessary.
In @.claude/skills/react-best-practices/references/rules/js-hoist-regexp.md:
- Around line 25-35: The example is missing the escapeRegex helper and includes
an unused EMAIL_REGEX that will confuse readers; add a small escapeRegex utility
(referenced by Highlighter and the useMemo call creating regex) so
escapeRegex(query) is defined and used, and either remove or move EMAIL_REGEX
into a separate, self-contained example (or comment that it’s unrelated) to
avoid confusion—ensure the Highlighter example references only escapeRegex and
keep EMAIL_REGEX shown elsewhere with a clear label.
In @.claude/skills/react-best-practices/references/rules/rendering-hoist-jsx.md:
- Around line 28-42: The examples reference an undefined loading variable;
update the examples to define or obtain loading (e.g., declare a local const
loading = true/false, accept loading as a prop on Container, or derive it from a
hook like useIsFetching) so the Container and loadingSkeleton examples are
executable; modify the Container function (and any usage of loadingSkeleton) to
use that defined loading value.
- Around line 12-26: Update the "Incorrect" example to demonstrate JSX being
recreated inside the component body rather than using a separate component
function; replace or remove the LoadingSkeleton component and instead show
Container defining a local JSX value (e.g. const loadingSkeleton = <div
className="animate-pulse h-20 bg-gray-200" />) and using {loading &&
loadingSkeleton} so the example clearly illustrates recreation on every render
(refer to the LoadingSkeleton and Container examples and the loadingSkeleton
variable to locate the code to change).
In
@.claude/skills/react-best-practices/references/rules/rerender-defer-reads.md:
- Around line 8-10: Update the "Defer State Reads to Usage Point" guidance to
include critical context clarifying when to apply the pattern: state reads
(e.g., searchParams, localStorage) should only be deferred if the component's UI
does not need to react to changes (purely event-driven reads), the code runs in
a client-side context (not SSR or RSC), and bypassing the framework routing
system is acceptable; explicitly call out these three constraints and warn that
components needing reactive updates should not use this optimization.
In
@.claude/skills/react-best-practices/references/rules/rerender-dependencies.md:
- Around line 38-44: The example only enables mobile mode when isMobile becomes
true but never disables it when isMobile becomes false; update the useEffect to
handle both transitions by calling the appropriate API to set mobile mode to the
current state (e.g., replace enableMobileMode() with a call that accepts the
boolean or call setMobileMode(isMobile)), ensuring the effect uses isMobile in
its dependency array so toggling width enables and disables mobile mode
correctly in the useEffect associated with isMobile.
In @.claude/skills/react-best-practices/references/rules/server-cache-react.md:
- Around line 1-26: Add a short "Requirements" note clarifying that
React.cache() (the cache import from 'react' used in getCurrentUser) requires
React 19+ and only works in Server Components; explicitly state it will not work
in Client Components or with React 18 to prevent runtime errors and confusion,
and place this note directly after the title/metadata so readers see
compatibility constraints before the usage example.
In @.claude/skills/react-best-practices/SKILL.md:
- Around line 113-115: Update the heading and occurrences to hyphenate
"data-fetching": change "### 4. Client-Side Data Fetching (MEDIUM-HIGH)" to "###
4. Client-Side Data‑Fetching (MEDIUM-HIGH)" and replace the inline "data
fetching" phrase in the list item ("Automatic deduplication and efficient data
fetching patterns.") with "data‑fetching" to ensure consistent hyphenation;
locate these strings by searching for the exact heading "Client-Side Data
Fetching" and the sentence "Automatic deduplication and efficient data fetching
patterns." and apply the hyphenated form.
In `@src/lib/yieldxyz/providerDescriptions.ts`:
- Around line 1-46: PROVIDER_DESCRIPTIONS currently hardcodes user-facing
strings; move each provider description into the translation files under the
yieldXYZ namespace (e.g. keys like "yieldXYZ.providers.morpho.description"),
replace the description fields in PROVIDER_DESCRIPTIONS with a descriptionKey
(e.g. "yieldXYZ.providers.morpho.description"), and update the consumer
(YieldProviderInfo.tsx) to call useTranslate().t(descriptionKey) (or
t(descriptionKey)) when rendering the description; ensure keys match between the
translation JSON and the descriptionKey values and keep provider ids (morpho,
morpho-aave, lido, etc.) unchanged.
In `@src/pages/Yields/components/YieldPositionCard.tsx`:
- Around line 131-136: The claimable/pending UI is rendered whenever
claimableBalance exists even if its amount is zero; update the guard used for
rendering (currently hasClaimable) to check the aggregated amount or canClaim
instead of mere presence. Specifically, change hasClaimable to verify
bnOrZero(claimableBalance?.aggregatedAmount).gt(0) or use
claimableBalance?.canClaim (whichever semantic fits) and then use that updated
boolean where the pending/claimable section is rendered so zero-amount
claimables do not show.
🧹 Nitpick comments (30)
src/components/Layout/Header/NavBar/NavigationDropdown.tsx (1)
140-153: LGTM - Badge implementation is clean.The
HStackwrapper with conditional badge rendering follows Chakra patterns well and uses proper translation for the "New" label. The badge styling is consistent with the design system.Minor optional consideration: The
HStackis now always rendered even whenisNewis false. If you want to avoid the extra DOM node, you could conditionally wrap:♻️ Optional: Avoid HStack when no badge
- <HStack spacing={2}> - <Text>{translate(item.label)}</Text> - {item.isNew && ( - <Badge - colorScheme='blue' - fontSize='2xs' - variant='solid' - borderRadius='full' - px={1.5} - > - {translate('common.new')} - </Badge> - )} - </HStack> + {item.isNew ? ( + <HStack spacing={2}> + <Text>{translate(item.label)}</Text> + <Badge + colorScheme='blue' + fontSize='2xs' + variant='solid' + borderRadius='full' + px={1.5} + > + {translate('common.new')} + </Badge> + </HStack> + ) : ( + <Text>{translate(item.label)}</Text> + )}src/components/Layout/Header/Header.tsx (1)
86-95: Style objects recreated on every render.Per project coding guidelines on aggressive memoization, these style objects should be memoized or moved outside the component as constants since they have no dependencies.
♻️ Suggested fix: Move to module scope
+const searchBoxDisplay = { + base: 'none', + '2xl': 'flex', + xl: 'none', +} + +const iconButtonDisplay = { + base: 'flex', + '2xl': 'none', +} + export const Header = memo(() => { // ... - const searchBoxDisplay = { - base: 'none', - '2xl': 'flex', - xl: 'none', - } - - const iconButtonDisplay = { - base: 'flex', - '2xl': 'none', - }.claude/skills/react-best-practices/references/rules/js-early-exit.md (1)
14-50: Align the semantic behavior of both examples to clarify the teaching point.The "incorrect" and "correct" examples have different return semantics beyond just efficiency:
- Incorrect version (lines 14-32): Overwrites
errorMessageon each validation failure, ultimately returning the last error encountered across all users.- Correct version (lines 37-49): Returns immediately with the first error encountered.
This semantic difference (last error vs. first error) obscures the primary teaching point about unnecessary computation. Consider revising the incorrect example to accumulate the first error without overwriting, so both versions report the same error—then the only difference is efficiency.
📝 Suggested revision to align semantics
Modify the incorrect example to capture only the first error without overwriting:
function validateUsers(users: User[]) { let hasError = false let errorMessage = '' for (const user of users) { if (!user.email) { hasError = true - errorMessage = 'Email required' + if (!errorMessage) errorMessage = 'Email required' } if (!user.name) { hasError = true - errorMessage = 'Name required' + if (!errorMessage) errorMessage = 'Name required' } // Continues checking all users even after error found } return hasError ? { valid: false, error: errorMessage } : { valid: true } }Alternatively, add a note in the documentation acknowledging that the incorrect version reports the last error while the correct version reports the first error, making it clear that the behavior differs in more ways than just performance.
.claude/skills/react-best-practices/references/rules/js-hoist-regexp.md (1)
37-43: Make the global regex warning more actionable.The warning accurately documents the mutable
lastIndexissue, but doesn't guide developers on how to handle it. Consider adding remediation options.💡 Suggested addition
**Warning:** Global regex (`/g`) has mutable `lastIndex` state: ```typescript const regex = /foo/g regex.test('foo') // true, lastIndex = 3 regex.test('foo') // false, lastIndex = 0
+Solutions:
+
+typescript +// Option 1: Omit /g flag when not needed +const regex = /foo/i // stateless + +// Option 2: Reset lastIndex before each use +const regex = /foo/g +regex.lastIndex = 0 +regex.test('foo') // predictable + +// Option 3: Create a new regex for each use +const regex = useMemo(() => new RegExp('foo', 'g'), []) +</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/client-event-listeners.md (1)</summary><blockquote> `58-62`: **Add error isolation when dispatching callbacks.** Line 60 dispatches all callbacks without error handling. If one callback throws, subsequent callbacks in the Set won't execute. <details> <summary>🛡️ Proposed error isolation</summary> ```diff const handler = (e: KeyboardEvent) => { if (e.metaKey && keyCallbacks.has(e.key)) { - keyCallbacks.get(e.key)!.forEach(cb => cb()) + keyCallbacks.get(e.key)!.forEach(cb => { + try { + cb() + } catch (error) { + console.error('Keyboard shortcut callback failed:', error) + } + }) } }.claude/skills/react-best-practices/references/rules/js-tosorted-immutable.md (1)
14-23: Consider clarifying the mutation timing in the "incorrect" example.The example correctly demonstrates the problem, but readers might wonder "when does the mutation happen?" since it's inside
useMemo. Adding a brief note that the mutation occurs each time the dependency changes (i.e., wheneverusersreference changes) would make the issue clearer.📝 Suggested clarification
**Incorrect (mutates original array):** ```typescript function UserList({ users }: { users: User[] }) { - // Mutates the users prop array! + // Mutates the users prop array whenever the dependency changes! + // This violates React's immutability contract const sorted = useMemo( () => users.sort((a, b) => a.name.localeCompare(b.name)), [users] ) return <div>{sorted.map(renderUser)}</div> }</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/rerender-defer-reads.md (1)</summary><blockquote> `5-5`: **Consider adding localStorage example or removing from tags.** The tags include `localStorage` but the document only demonstrates the pattern with `searchParams`. For completeness, consider either: 1. Adding a localStorage example showing the same pattern (reading in event handler vs subscribing to storage events) 2. Removing localStorage from the tags if out of scope <details> <summary>💡 Example localStorage addition</summary> ```markdown **Incorrect (subscribes to all localStorage changes):** ```tsx function SaveButton({ data }: { data: Data }) { const [savedCount, setSavedCount] = useState(() => Number(localStorage.getItem('saveCount') || '0') ) useEffect(() => { const handleStorageChange = () => { setSavedCount(Number(localStorage.getItem('saveCount') || '0')) } window.addEventListener('storage', handleStorageChange) return () => window.removeEventListener('storage', handleStorageChange) }, []) const handleSave = () => { save(data) localStorage.setItem('saveCount', String(savedCount + 1)) } return <button onClick={handleSave}>Save</button> }Correct (reads on demand, no subscription):
function SaveButton({ data }: { data: Data }) { const handleSave = () => { const currentCount = Number(localStorage.getItem('saveCount') || '0') save(data) localStorage.setItem('saveCount', String(currentCount + 1)) } return <button onClick={handleSave}>Save</button> }</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/rendering-svg-precision.md (2)</summary><blockquote> `8-10`: **Add guidance on visual trade-offs and precision selection.** The introduction correctly notes that optimal precision depends on viewBox size but doesn't explain the relationship or warn about visual impact. Consider adding: - How to choose precision based on viewBox dimensions (e.g., viewBox 0-100 vs 0-1000) - Warning that reducing precision changes the visual output - When to avoid aggressive reduction (intricate paths, icons requiring pixel-perfect rendering at specific sizes) <details> <summary>📝 Suggested enhancement</summary> ```diff ## Optimize SVG Precision -Reduce SVG coordinate precision to decrease file size. The optimal precision depends on the viewBox size, but in general reducing precision should be considered. +Reduce SVG coordinate precision to decrease file size. The optimal precision depends on the viewBox size—larger viewBoxes (e.g., 0-1000) tolerate lower precision than smaller ones (e.g., 0-100). + +**Important:** Reducing precision alters coordinates and may change the visual output. Test the result to ensure the change is imperceptible. Avoid aggressive reduction for intricate paths or icons that must be pixel-perfect at specific sizes.
12-22: Consider more nuanced framing of precision examples.The labels "Incorrect (excessive precision)" vs "Correct (1 decimal place)" are too absolute. High precision isn't incorrect—it's just inefficient for most use cases. Additionally, presenting 1 decimal place as universally "correct" without context may mislead readers.
📝 Suggested refinement
-**Incorrect (excessive precision):** +**Before (excessive precision for most use cases):** ```svg <path d="M 10.293847 20.847362 L 30.938472 40.192837" />-Correct (1 decimal place):
+After (optimized to 1 decimal place):<path d="M 10.3 20.8 L 30.9 40.2" />
+This reduction is typically imperceptible but saves bytes. Adjust precision based on viewBox scale and visual requirements.
</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/js-batch-dom-css.md (1)</summary><blockquote> `26-39`: **Mixed CSS and JavaScript syntax in single code block.** The code block at lines 27-38 combines CSS (`.highlighted-box`) and JavaScript (`function updateElementStyles`) under a TypeScript identifier. This may cause syntax highlighting issues in some editors/viewers. Consider splitting into two separate blocks. <details> <summary>Suggested split</summary> ```diff -```typescript -// CSS file -.highlighted-box { - width: 100px; - height: 200px; - background-color: blue; - border: 1px solid black; -} - -// JavaScript -function updateElementStyles(element: HTMLElement) { - element.classList.add('highlighted-box') -} -``` +```css +/* styles.css */ +.highlighted-box { + width: 100px; + height: 200px; + background-color: blue; + border: 1px solid black; +} +``` + +```typescript +function updateElementStyles(element: HTMLElement) { + element.classList.add('highlighted-box') +} +```.claude/skills/react-best-practices/references/rules/bundle-dynamic-imports.md (1)
24-35: Consider adding React.lazy alternative for non-Next.js contexts.The example uses
next/dynamicwhich is Next.js specific. Per learnings, the project also usesReact.lazyfor code splitting. Consider documenting both patterns or clarifying when each applies.Add React.lazy alternative
const MonacoEditor = dynamic( () => import('./monaco-editor').then(m => m.MonacoEditor), { ssr: false } ) function CodePanel({ code }: { code: string }) { return <MonacoEditor value={code} /> }
+Alternative (React.lazy with Suspense):
+
+```tsx
+import { lazy, Suspense } from 'react'
+
+const MonacoEditor = lazy(() =>
- import('./monaco-editor').then(m => ({ default: m.MonacoEditor }))
+)+function CodePanel({ code }: { code: string }) {
- return (
- <Suspense fallback={
Loading editor...}><MonacoEditor value={code} />- )
+}
+```</details> </blockquote></details> <details> <summary>.claude/skills/react-best-practices/references/rules/rendering-content-visibility.md (1)</summary><blockquote> `38-38`: **Consider adding guidance on when to prefer virtualization.** Per project learnings, virtualization is recommended for lists with 100+ items. `content-visibility` is a lighter-weight alternative but doesn't reduce DOM node count. Consider adding a note clarifying when to use each approach—e.g., virtualization for very large lists where memory/DOM size matters, `content-visibility` for moderate lists where simpler implementation is preferred. </blockquote></details> <details> <summary>src/pages/Yields/components/YieldExplainers.tsx (2)</summary><blockquote> `99-110`: **Consider using a stable key instead of array index.** Using `index` as the key works but can cause issues if the explainers list changes dynamically. Since `textKey` is unique per explainer item, it would be a more stable key. <details> <summary>Suggested improvement</summary> ```diff - <HStack key={index} spacing={3} align='flex-start'> + <HStack key={explainer.textKey} spacing={3} align='flex-start'>
19-72: Create a type guard for mechanics type validation.Since
mechanics.typeis defined asstringrather than a discriminated union, TypeScript won't enforce exhaustiveness checks if new types are added. Consider creating a type guard function to document and validate the supported mechanics types at runtime, helping prevent silent failures if new types are introduced:const isSupportedMechanicsType = (type: string): type is 'liquid-staking' | 'native-staking' | 'pooled-staking' | 'staking' | 'restaking' | 'vault' | 'lending' => { return ['liquid-staking', 'native-staking', 'pooled-staking', 'staking', 'restaking', 'vault', 'lending'].includes(type) }This approach makes the supported types explicit and enables better IDE support for refactoring.
.claude/skills/react-best-practices/references/rules/async-defer-await.md (1)
62-78: Documentation is well-structured but consider noting the trade-off.The "correct" example defers
fetchPermissionsuntil after checking if the resource exists, which is good. However, it's worth noting that the optimal order depends on which check is cheaper/more likely to short-circuit. In this example, if most resources exist but few users have edit permissions, fetching permissions first might be better.Consider adding a brief note about evaluating which check to perform first based on:
- Cost of each operation
- Likelihood of each condition failing
This is optional since the core pattern is correctly demonstrated.
.claude/skills/react-best-practices/references/rules/rerender-transitions.md (1)
14-23: Consider clarifying the "blocking" terminology.The "incorrect" example doesn't technically "block" the UI (the event handler returns immediately). Rather, it triggers synchronous high-priority re-renders on every scroll event, which can cause jank in complex UIs. Consider rephrasing to "causes frequent synchronous re-renders" for technical accuracy.
Also, both examples are missing the
useStateimport for completeness..claude/skills/react-best-practices/references/rules/js-cache-property-access.md (3)
14-18: Consider modern loop patterns to align with project conventions.The example uses
let iin a traditional for loop, which conflicts with the project's preference to avoidletassignments. Consider demonstrating with afor...ofloop or array methods:for (const item of arr) { process(obj.config.settings.value) }This maintains readability while following project conventions.
Based on learnings, the project prefers avoiding
letvariable assignments.
12-12: Clarify the "3 lookups" claim.The statement "3 lookups × N iterations" may be misleading—each iteration performs one property chain traversal (
obj.config.settings.value), not three separate lookups. While the chain involves three property accesses internally, modern JS engines optimize this. Consider rephrasing to "N property chain traversals" for accuracy.
23-27: Apply consistent loop pattern in the correct example.For consistency with the suggestion above, consider using
for...ofhere as well:const value = obj.config.settings.value for (const _ of arr) { process(value) }This demonstrates the caching benefit without the
letdeclaration..claude/skills/react-best-practices/references/rules/async-api-routes.md (1)
38-38: Consider adding context for thebetter-alltool reference.The reference to "Dependency-Based Parallelization" and the
better-alltool lacks context. Consider either:
- Adding a link or brief explanation of what
better-allis- Including this information in the referenced section
- Providing a code example showing its usage
This will help readers evaluate whether they need this additional tool.
.claude/skills/react-best-practices/references/rules/rendering-hydration-no-flicker.md (1)
80-82: Document React version compatibility and modern alternatives.Consider adding a note that:
- This inline script pattern may produce hydration warnings in React 18+
- React 19 offers better alternatives (streaming SSR with Suspense,
use clientdirective)- This pattern is primarily useful for legacy React applications
This helps readers make informed decisions based on their React version.
.claude/skills/react-best-practices/references/rules/advanced-event-handler-refs.md (1)
8-38: Consider documentinguseEffectEventas the modern solution.The ref pattern shown is correct, but React 19.2 introduced
useEffectEventspecifically for this use case. Consider adding it as the preferred approach with the ref pattern as a fallback for older versions:// Modern approach (React 19.2+) function useWindowEvent(event: string, handler: () => void) { const stableHandler = useEffectEvent(handler) useEffect(() => { window.addEventListener(event, stableHandler) return () => window.removeEventListener(event, stableHandler) }, [event]) }The current ref pattern remains valid as a fallback for projects not yet on React 19.2.
Based on React 19 documentation showing useEffectEvent is now stable.
.claude/skills/react-best-practices/references/rules/rendering-animate-svg-wrapper.md (1)
14-45: Documentation assumes Tailwind CSS usage.The examples use
className="animate-spin", which is a Tailwind CSS utility class. If the project doesn't use Tailwind CSS, consider adding a note or providing a vanilla CSS alternative for the animation.📝 Optional: Add vanilla CSS alternative
You could add a note like:
**Note:** The examples use Tailwind CSS (`animate-spin`). If not using Tailwind, add this CSS: ```css `@keyframes` spin { from { transform: rotate(0deg); } to { transform: rotate(360deg); } } .animate-spin { animation: spin 1s linear infinite; }.claude/skills/react-best-practices/references/rules/rerender-lazy-state-init.md (1)
37-43: Consider clarifying that lazy initializers don't track prop changes.The
FilteredListexample usesitemsprop in the lazy initializer, butuseStateonly runs this once. Ifitemschanges after mount,searchIndexwon't update. This could mislead readers into thinking lazy initialization handles prop-derived state.Consider either:
- Adding a note that this pattern is for truly static initial values, or
- Using an example without prop dependencies (like the localStorage one)
src/lib/yieldxyz/getYieldDisplayName.test.ts (1)
6-11: Add an explicit return type formockYield.This keeps the test helper aligned with the project’s TS typing rules and avoids implicit return types. As per coding guidelines, explicit return types are required in TypeScript helpers.
♻️ Proposed change
-const mockYield = (providerId: string, tokenSymbol: string, metadataName: string) => +const mockYield = (providerId: string, tokenSymbol: string, metadataName: string): AugmentedYieldDto => ({ providerId, token: { symbol: tokenSymbol }, metadata: { name: metadataName }, }) as AugmentedYieldDtosrc/pages/Yields/components/YieldItem.tsx (1)
78-110: Consider memoizing thestatscomputation.The
statsobject is computed on every render via an IIFE. Since this involves array operations (map,reduce,filter) for group yields and creates a new object each time, it could cause unnecessary re-renders of child components. Given the team's aggressive memoization practice, wrapping this inuseMemowith[data, yieldProviders]dependencies would be more consistent with codebase conventions.♻️ Suggested refactor
- const stats = (() => { + const stats = useMemo(() => { if (isSingle) { const y = data.yieldItem return { apy: y.rewardRate.total, apyLabel: y.rewardRate.rateType, tvlUsd: y.statistics?.tvlUsd ?? '0', providers: [{ id: y.providerId, logo: data.providerIcon }], chainIds: y.chainId ? [y.chainId] : [], count: 1, name: y.metadata.name, canEnter: y.status.enter, } } const yields = data.yields const maxApy = Math.max(0, ...yields.map(y => y.rewardRate.total)) const totalTvlUsd = yields .reduce((acc, y) => acc.plus(bnOrZero(y.statistics?.tvlUsd)), bnOrZero(0)) .toFixed() const providerIds = [...new Set(yields.map(y => y.providerId))] const chainIds = [...new Set(yields.map(y => y.chainId).filter(Boolean))] as string[] return { apy: maxApy, apyLabel: 'APY', tvlUsd: totalTvlUsd, providers: providerIds.map(id => ({ id, logo: yieldProviders?.[id]?.logoURI })), chainIds, count: yields.length, name: data.assetName, canEnter: true, } - })() + }, [data, isSingle, yieldProviders])src/pages/Yields/components/YieldStats.tsx (1)
47-66: Consider memoizing validator lookups for consistency.The
selectedValidatorAddressandselectedValidatorcomputations involve arrayfindoperations that run on every render. While the performance impact is likely minimal for a single-instance component, memoizing these would align with the codebase's aggressive memoization conventions.♻️ Optional refactor for consistency
+ import { memo, useMemo } from 'react' - import { memo } from 'react' - const selectedValidatorAddress = (() => { + const selectedValidatorAddress = useMemo(() => { if ( yieldItem.id === COSMOS_ATOM_NATIVE_STAKING_YIELD_ID || yieldItem.id === SOLANA_SOL_NATIVE_MULTIVALIDATOR_STAKING_YIELD_ID || (yieldItem.id.includes('solana') && yieldItem.id.includes('native')) ) { return defaultValidator } return validatorParam || defaultValidator - })() + }, [yieldItem.id, defaultValidator, validatorParam]) - const selectedValidator = (() => { + const selectedValidator = useMemo(() => { if (!selectedValidatorAddress) return undefined const inList = validators?.find(v => v.address === selectedValidatorAddress) if (inList) return inList const inBalances = balances?.raw.find(b => b.validator?.address === selectedValidatorAddress) ?.validator if (inBalances) return inBalances return undefined - })() + }, [selectedValidatorAddress, validators, balances?.raw])src/pages/Yields/components/YieldOpportunityStats.tsx (1)
56-123: Memoize derived totals to avoid repeated reductions each render.
activeValueUsd,idleValueUsd, and weighted APY/earnings calculations iterate over arrays and rebuild Sets on every render. Wrapping them inuseMemo(and reusing the memoized results for formatted strings) keeps renders cheap and aligns with the repo’s memoization policy. Also consider renamingidleValueUsdto reflect user-currency semantics to avoid confusion.As per coding guidelines, memoize derived values and object creations.♻️ Proposed refactor
-import { memo } from 'react' +import { memo, useMemo } from 'react' ... - const activeValueUsd = positions.reduce((acc, position) => { - const positionBalances = balances?.[position.id] - if (!positionBalances) return acc - const activeBalances = positionBalances.filter(b => b.type === 'active' || b.type === 'locked') - return activeBalances.reduce((sum, b) => sum.plus(bnOrZero(b.amountUsd)), acc) - }, bnOrZero(0)) + const activeValueUsd = useMemo(() => { + return positions.reduce((acc, position) => { + const positionBalances = balances?.[position.id] + if (!positionBalances) return acc + const activeBalances = positionBalances.filter(b => b.type === 'active' || b.type === 'locked') + return activeBalances.reduce((sum, b) => sum.plus(bnOrZero(b.amountUsd)), acc) + }, bnOrZero(0)) + }, [positions, balances]) ... - const idleValueUsd = (() => { + const idleValueUsd = useMemo(() => { if (!isConnected || !allYields) return bnOrZero(0) ... - })() + }, [isConnected, allYields, portfolioBalances]) ... - const { weightedApy, potentialEarningsValue } = (() => { + const { weightedApy, potentialEarningsValue } = useMemo(() => { if (!isConnected || !yields?.byInputAssetId || !portfolioBalances) { return { weightedApy: 0, potentialEarningsValue: bnOrZero(0) } } ... - })() + }, [isConnected, yields?.byInputAssetId, portfolioBalances])src/pages/Yields/components/YieldInfoCard.tsx (1)
44-87: Memoize derived display values (icon source, APY, title).These values are deterministic from props and currently recompute each render, including JSX element creation. Memoizing them keeps render work stable and matches the project’s memoization guidance.
As per coding guidelines, memoize derived values and object creations.♻️ Proposed refactor
-import { memo } from 'react' +import { memo, useMemo } from 'react' ... - const iconSource = resolveYieldInputAssetIcon(yieldItem) - const apy = bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2) - const yieldTitle = titleOverride ?? getYieldDisplayName(yieldItem) + const iconSource = useMemo(() => resolveYieldInputAssetIcon(yieldItem), [yieldItem]) + const apy = useMemo( + () => bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2), + [yieldItem.rewardRate.total], + ) + const yieldTitle = useMemo( + () => titleOverride ?? getYieldDisplayName(yieldItem), + [titleOverride, yieldItem], + )src/pages/Yields/components/YieldHero.tsx (1)
56-85: Memoize derived values for render stability.Icon source, APY, and balance-derived flags are recomputed every render. Memoizing them keeps rendering cheaper and consistent with the repo’s aggressive memoization guidance.
As per coding guidelines, memoize derived values and object creations.♻️ Proposed refactor
- const iconSource = resolveYieldInputAssetIcon(yieldItem) - const apy = bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2) - const hasExitBalance = bnOrZero(userBalanceCrypto).gt(0) + const { iconSource, apy, hasExitBalance } = useMemo(() => { + return { + iconSource: resolveYieldInputAssetIcon(yieldItem), + apy: bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2), + hasExitBalance: bnOrZero(userBalanceCrypto).gt(0), + } + }, [yieldItem, userBalanceCrypto])
.claude/skills/react-best-practices/references/react-performance-guidelines.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/bundle-conditional.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/bundle-defer-third-party.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/client-event-listeners.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/rerender-dependencies.md
Show resolved
Hide resolved
.claude/skills/react-best-practices/references/rules/server-cache-react.md
Show resolved
Hide resolved
Fixes potential runtime error when allBalances[id] is undefined, which would cause .minus() to throw or produce NaN. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Check aggregatedAmount > 0 for hasClaimable, consistent with hasWithdrawable check, to avoid showing empty claimable UI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Description
A collection of UX and layout improvements for the yield/staking pages.
Layout & Display
YieldInfoCardcomponent for strategy info displayYieldAvailableToDepositcomponent showing wallet balance and potential earningsgetYieldDisplayNameutility for clean yield names (curator name for Morpho vaults, symbol for simple yields)UX Improvements
YieldExplainerscomponent for consistent staking infoBug Fixes
Issue (if applicable)
closes #
Risk
Low-Medium risk. UI/UX changes only, no changes to transaction logic or state management. Changes are isolated to yield pages.
None - these are purely display/UI changes.
Testing
Engineering
/yieldsand verify the list displays correctlyOperations
Manual testing steps:
Screenshots (if applicable)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.