Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces react-toastify's direct ToastContainer with a new ThemedToastContainer using next-themes; removes duplicate react-toastify CSS imports from toast hooks; migrates pendingDelta and related internal arithmetic from number to bigint across rebalance/position components; adds the Base network entry for sDAI in supportedTokens. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/components/ui/themed-toast-container.tsx:
- Around line 7-17: The ThemedToastContainer currently reads resolvedTheme from
useTheme on first render causing a light-theme flash; update
ThemedToastContainer to avoid rendering until the client has mounted (e.g.,
track a local mounted state and only return the <ToastContainer> when mounted)
or export it via next/dynamic with ssr: false so it only renders on the client;
ensure you reference resolvedTheme from useTheme and pass theme to
ToastContainer only after mounted to prevent the initial undefined -> light
fallback.
In @src/utils/tokens.ts:
- Around line 264-267: In src/utils/tokens.ts update the networks entry for the
sDAI token inside the networks array to use the correct Base address: replace
the incorrect '0x50c5725949a6f0c72e6c4a641f24049a917db0cb' with
'0x99ac4484e8a1dbd6a185380b3a811913ac884d87' so the sDAI token’s Base network
mapping is accurate; locate the sDAI token object in the tokens list (the
networks: [...] block) and modify the Base address accordingly.
🧹 Nitpick comments (2)
src/features/positions/components/rebalance/rebalance-modal.tsx (1)
56-69: Remove redundant BigInt() wrappers.
action.amountis alreadybigintper the RebalanceAction type definition. TheBigInt()calls on lines 60 and 63 are unnecessary.♻️ Simplify by removing redundant conversions
const getPendingDelta = useCallback( (marketUniqueKey: string): bigint => { return rebalanceActions.reduce((acc: bigint, action: RebalanceAction) => { if (action.fromMarket.uniqueKey === marketUniqueKey) { - return acc - BigInt(action.amount); + return acc - action.amount; } if (action.toMarket.uniqueKey === marketUniqueKey) { - return acc + BigInt(action.amount); + return acc + action.amount; } return acc; }, 0n); }, [rebalanceActions], );src/features/positions/components/from-markets-table.tsx (1)
140-144: Consider using formatUnits for consistency.The display calculation converts bigint values to numbers before dividing, which could lose precision for large amounts. The pattern in
rebalance-action-row.tsxusesformatUnitsfirst (which preserves bigint precision) thenformatReadable.♻️ More precise display calculation
<div> - {formatReadable( - (Number(position.state.supplyAssets) + Number(position.pendingDelta)) / - 10 ** position.market.loanAsset.decimals, - )}{' '} + {formatReadable( + formatUnits( + BigInt(position.state.supplyAssets) + position.pendingDelta, + position.market.loanAsset.decimals + ) + )}{' '} {position.market.loanAsset.symbol} </div>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/providers/ThemeProvider.tsxsrc/components/ui/themed-toast-container.tsxsrc/features/positions/components/from-markets-table.tsxsrc/features/positions/components/rebalance/rebalance-action-row.tsxsrc/features/positions/components/rebalance/rebalance-modal.tsxsrc/hooks/useStyledToast.tsxsrc/hooks/useTransactionWithToast.tsxsrc/utils/tokens.ts
💤 Files with no reviewable changes (2)
- src/hooks/useStyledToast.tsx
- src/hooks/useTransactionWithToast.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-17T12:39:21.949Z
Learnt from: antoncoding
Repo: antoncoding/monarch PR: 99
File: app/positions/components/onboarding/SuccessPage.tsx:17-17
Timestamp: 2024-12-17T12:39:21.949Z
Learning: Prefer using explicit network checks like `selectedToken?.network === SupportedNetworks.Base` over using array indexing like `config.agentSupportedNetworks[0]` for better maintainability.
Applied to files:
src/utils/tokens.ts
🧬 Code graph analysis (4)
src/features/positions/components/rebalance/rebalance-action-row.tsx (1)
src/utils/balance.ts (1)
formatReadable(25-49)
src/components/providers/ThemeProvider.tsx (1)
src/components/ui/themed-toast-container.tsx (1)
ThemedToastContainer(7-17)
src/features/positions/components/from-markets-table.tsx (2)
src/utils/types.ts (1)
MarketPosition(12-21)src/utils/morpho.ts (1)
previewMarketState(294-355)
src/features/positions/components/rebalance/rebalance-modal.tsx (1)
src/utils/types.ts (1)
RebalanceAction(195-215)
🔇 Additional comments (6)
src/features/positions/components/rebalance/rebalance-action-row.tsx (1)
8-8: LGTM on the display formatting approach.The conditional formatting distinguishes input mode (preserves raw string for editing) from display mode (applies formatReadable for presentation). Using formatUnits then formatReadable maintains precision throughout the bigint conversion.
Also applies to: 79-81
src/features/positions/components/rebalance/rebalance-modal.tsx (1)
110-110: LGTM on bigint migration.The updates to
pendingBalance,handleMaxSelect, andisMaxAmountcorrectly handle bigint arithmetic and maintain type consistency throughout the rebalance logic.Also applies to: 150-160, 178-182
src/features/positions/components/from-markets-table.tsx (1)
13-13: LGTM on bigint type migration.The updates to
PositionWithPendingDelta,onSelectMaxsignature, and all bigint arithmetic operations are correct and maintain type consistency withpreviewMarketStateexpectations.Also applies to: 19-19, 33-36, 69-72, 150-155
src/components/ui/themed-toast-container.tsx (1)
1-5: LGTM!The 'use client' directive is correctly placed for a component using hooks. Centralizing the CSS import here is good practice.
src/components/providers/ThemeProvider.tsx (2)
4-4: Clean refactor.Delegating toast rendering to a dedicated component improves separation of concerns.
17-17: LGTM!Correct placement inside NextThemesProvider allows the toast container to access theme context.
| export function ThemedToastContainer() { | ||
| const { resolvedTheme } = useTheme(); | ||
|
|
||
| return ( | ||
| <ToastContainer | ||
| position="bottom-right" | ||
| theme={resolvedTheme === 'dark' ? 'dark' : 'light'} | ||
| toastStyle={{ borderRadius: '3px', fontSize: '16px' }} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nP '\bresolvedTheme\b' --type=ts --type=tsx -C3Repository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
# Also check for suppressHydrationWarning or other hydration handling patterns
rg -i 'suppressHydrationWarning|hydration' --type=ts --type=tsx | head -20Repository: antoncoding/monarch
Length of output: 90
🌐 Web query:
next-themes resolvedTheme undefined hydration initial render
💡 Result:
Short answer
- resolvedTheme is often undefined on initial render because the server can't access browser state (localStorage / prefers-color-scheme). next-themes returns undefined until the component mounts on the client, which causes hydration mismatches. [1][2]
How to fix (pick one)
- Wait for client mount: render theme-dependent UI only after mounting.
- const [mounted, setMounted] = useState(false);
- useEffect(() => setMounted(true), []);
- if (!mounted) return null (or a neutral placeholder). [1][2]
- Client-only component: lazy-load the theme-dependent component with next/dynamic({ ssr: false }). [1][4]
- Rely on ThemeProvider's no-flash script: ensure ThemeProvider is configured (attribute/class, enableColorScheme, storageKey) so it injects the pre-paint script that sets the HTML attribute and avoids flashing; still avoid reading resolvedTheme on server. [1][2]
- For SSR-synced solution: determine theme on the server (cookie or server-side logic) and render that on initial HTML so server and client match. [5]
Notes
- These fixes address hydration errors documented by next-themes and Next.js. If you want, tell me which approach you prefer and share the relevant code and I’ll give a patched snippet. [1][2][3]
Sources
[1] next-themes README / examples (avoiding hydration mismatch, resolvedTheme examples).
[2] next-themes GitHub (themed images / README discussion).
[3] Next.js docs on hydration errors and fixes.
[4] next-themes README: example using next/dynamic({ ssr: false }).
[5] community posts / articles on SSR approaches to theming.
🏁 Script executed:
rg '\bresolvedTheme\b' --type=typescript -C3Repository: antoncoding/monarch
Length of output: 97
🏁 Script executed:
# Also check for useTheme hook usage patterns
rg 'useTheme\(\)' --type=typescript -C2Repository: antoncoding/monarch
Length of output: 97
🏁 Script executed:
# Check if there's a ThemeProvider setup in the repo
fd -e ts -e tsx | head -20 && echo "---" && git ls-files | grep -E '\.(ts|tsx)$' | head -20Repository: antoncoding/monarch
Length of output: 1102
🏁 Script executed:
# Look for the _app or layout file to see ThemeProvider configuration
find . -name '_app.*' -o -name 'layout.*' -o -name 'app.*' | grep -E '\.(ts|tsx)$' | head -10Repository: antoncoding/monarch
Length of output: 79
🏁 Script executed:
# Use basic grep approach instead
grep -r "resolvedTheme" --include="*.ts" --include="*.tsx" -nRepository: antoncoding/monarch
Length of output: 483
🏁 Script executed:
# Check the layout.tsx and ThemedToastContainer directly
cat app/layout.tsxRepository: antoncoding/monarch
Length of output: 1949
🏁 Script executed:
cat src/components/ui/themed-toast-container.tsxRepository: antoncoding/monarch
Length of output: 491
🏁 Script executed:
cat src/components/providers/ThemeProvider.tsxRepository: antoncoding/monarch
Length of output: 674
Handle resolvedTheme hydration to prevent theme flash on initial load.
When the component first renders on the client, resolvedTheme is undefined, which defaults this to the light theme. Dark mode users will see a brief light theme flash before hydration completes. Wrap the component to only render after the client has mounted, or use next/dynamic({ ssr: false }) to skip server rendering entirely.
🤖 Prompt for AI Agents
In @src/components/ui/themed-toast-container.tsx around lines 7 - 17, The
ThemedToastContainer currently reads resolvedTheme from useTheme on first render
causing a light-theme flash; update ThemedToastContainer to avoid rendering
until the client has mounted (e.g., track a local mounted state and only return
the <ToastContainer> when mounted) or export it via next/dynamic with ssr: false
so it only renders on the client; ensure you reference resolvedTheme from
useTheme and pass theme to ToastContainer only after mounted to prevent the
initial undefined -> light fallback.
|
nice work |
2 participants
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.