Conversation
📝 WalkthroughWalkthroughNode/tooling upgraded, package manager switched, and Tailwind theme moved to HeroUI. Project-wide UI migration from NextUI to HeroUI with Button API shifting to onPress. Added error boundaries. Markets page now parses search params server-side and passes initial props. Subgraph fetchers gained graceful error handling. Providers/wallet config refactored. Misc SSR/typing tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NextApp as Next.js App Router
participant Page as Page/Server Component
participant Markets as Markets (Client)
Note over NextApp,Page: Markets route
User->>NextApp: Navigate /markets?network=...&collaterals=...&loanAssets=...
NextApp->>Page: Call MarketPage({ searchParams })
Page->>Page: Parse/validate params (server)
Page->>Markets: Render with {initialNetwork, initialCollaterals, initialLoanAssets}
Markets->>Markets: Init state (SSR-safe useLocalStorage)
User->>Markets: Interact (filter/sort)
Markets->>NextApp: router.push(updated query)
sequenceDiagram
autonumber
participant React as React Runtime
participant ErrorBoundary as app/error.tsx
participant GlobalError as app/global-error.tsx
participant User as User
Note over React,ErrorBoundary: Client error handling
React-->>ErrorBoundary: Render with error
ErrorBoundary->>ErrorBoundary: useEffect(console.error)
User->>ErrorBoundary: Click "Retry"
ErrorBoundary->>React: reset() to re-render subtree
Note over React,GlobalError: Unhandled/app-level errors render GlobalError similarly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.nvmrc (1)
1-1: Optional: consider pinning strategy.If you want reproducibility, pin the full patch (e.g., 22.12.0). If you prefer auto-updating to the latest LTS, use lts/* in .nvmrc. Current style (major.minor) is OK if that’s intentional.
package.json (1)
90-90: Type bump to @types/node ^22 approved—verify type-check
- tsconfig.json currently has
lib: ["dom","dom.iterable","esnext"], no explicittypes.- No obvious DOM/Node global name conflicts found in source.
- Run
yarn tsc --noEmitto confirm there are no new errors.- If you hit DOM vs Node type clashes, tighten your config to:
•compilerOptions.lib: ["es2022"]
•compilerOptions.types: ["node","jest"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.nvmrc(1 hunks)package.json(1 hunks)
🔇 Additional comments (1)
.nvmrc (1)
1-1: No other Node version pins found—Node 22 bump OKI ran the provided script and didn’t find any Node versions pinned in package.json, workflows, Dockerfiles or platform configs. Ready to roll with Node 22.
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (5)
app/positions/components/FromAndToMarkets.tsx (1)
291-300: Bug: PressEvent doesn’t have stopPropagation; this will throwNextUI’s onPress receives a React Aria PressEvent, which doesn’t expose stopPropagation. Clicking “Max” will error at runtime.
Recommended fix (remove event usage; row onClick stays idempotent):
- onPress={(e) => { - e.stopPropagation(); - onFromMarketSelect(marketPosition.market.uniqueKey); - if (maxTransferableAmount > 0n) { - onSelectMax?.( - marketPosition.market.uniqueKey, - Number(maxTransferableAmount), - ); - } - }} + onPress={() => { + onFromMarketSelect(marketPosition.market.uniqueKey); + if (maxTransferableAmount > 0n) { + onSelectMax?.( + marketPosition.market.uniqueKey, + Number(maxTransferableAmount), + ); + } + }}If you must block the row’s onClick, alternatively switch this one control back to onClick:
- onPress={(e) => { - e.stopPropagation(); + onClick={(e) => { + e.stopPropagation(); onFromMarketSelect(marketPosition.market.uniqueKey); if (maxTransferableAmount > 0n) { onSelectMax?.( marketPosition.market.uniqueKey, Number(maxTransferableAmount), ); } }}app/positions/components/PositionsSummaryTable.tsx (1)
366-385: Prevent row toggle when clicking “Rebalance”The row has onClick to expand (Line 266). Clicking the Rebalance button will also trigger the row’s onClick due to event bubbling. Stop propagation on the button to avoid accidental expand/collapse.
Apply this diff:
<Button size="sm" variant="interactive" className="text-xs" - onPress={() => { + onPress={() => { if (!isOwner) { toast.error( 'No authorization', 'You can only rebalance your own positions', ); return; } setSelectedGroupedPosition(groupedPosition); setShowRebalanceModal(true); }} + onClick={(e) => e.stopPropagation()} disabled={!isOwner} > Rebalance </Button>app/markets/components/MarketTableBody.tsx (1)
51-56: Prevent row toggle when clicking “Supply”The table row toggles onClick (Lines 52-55). The Supply button’s onPress doesn’t stop propagation, so clicks will also toggle the row. Stop propagation on the button.
Apply this diff:
<Button size="sm" variant="interactive" className="text-xs" - onPress={() => { + onPress={() => { setSelectedMarket(item); setShowSupplyModal(true); }} + onClick={(e) => e.stopPropagation()} > Supply </Button>Also applies to: 170-181
app/layout.tsx (1)
41-42: GoogleAnalytics is outside — invalid HTML structureThis can cause unpredictable rendering. Move the component inside body.
Apply this diff:
- </body> - <GoogleAnalytics /> + <GoogleAnalytics /> + </body>src/data-sources/subgraph/liquidations.ts (1)
36-41: Add$skipto the liquidation‐check query and guard against infinite loopsTo avoid fetching the same page endlessly, update both the GraphQL query and the pagination loops:
• In src/graphql/morpho-subgraph-queries.ts, extend
getSubgraphMarketsWithLiquidationCheckto accept and passskip:export const subgraphMarketsWithLiquidationCheckQuery = ` - query getSubgraphMarketsWithLiquidationCheck($first: Int, $where: Market_filter) { + query getSubgraphMarketsWithLiquidationCheck($first: Int, $skip: Int, $where: Market_filter) { markets( first: $first, + skip: $skip, where: $where ) { id liquidates { id } } } `• In src/data-sources/subgraph/liquidations.ts (around lines 36–41, 49–61 and 77–81), add a defensive break when no new keys are found:
while (true) { + const prevCount = liquidatedKeys.size; const variables = { first: pageSize, skip, where: { inputToken_not_in: blacklistTokens } }; const { markets } = await fetchSubgraph(subgraphMarketsWithLiquidationCheckQuery, variables); markets.forEach(market => { if (market.liquidates?.length) liquidatedKeys.add(market.id); }); if (markets.length < pageSize) break; + if (liquidatedKeys.size === prevCount) { + console.warn(`Pagination misconfigured at skip=${skip}. Breaking to avoid infinite loop.`); + break; + } skip += pageSize; }Repeat this guard in each pagination loop.
🧹 Nitpick comments (19)
app/market/[chainId]/[marketid]/content.tsx (1)
205-211: Harden window.open against reverse tabnabbingAdd noopener,noreferrer when opening a new tab.
- onPress={() => { - void window.open( - getMarketURL(market.uniqueKey, market.morphoBlue.chain.id), - '_blank', - ); - }} + onPress={() => { + void window.open( + getMarketURL(market.uniqueKey, market.morphoBlue.chain.id), + '_blank', + 'noopener,noreferrer', + ); + }}app/positions/components/RebalanceActionInput.tsx (1)
69-70: Optional: disable when amount <= 0Prevents an avoidable validation round-trip.
- isDisabled={!amount || !selectedFromMarketUniqueKey || !selectedToMarketUniqueKey} + isDisabled={ + !amount || + Number(amount) <= 0 || + !selectedFromMarketUniqueKey || + !selectedToMarketUniqueKey + }app/positions/components/onboarding/RiskSelection.tsx (1)
207-213: Minor: align Button import path for consistencyMost files import Button from '@/components/common'. This file uses '@/components/common/Button'. Consider aligning to the shared barrel to keep imports consistent.
If desired, update the import:
// before import { Button } from '@/components/common/Button'; // after import { Button } from '@/components/common';package.json (1)
98-102: Align ESLint core with eslint-config-next 15eslint-config-next@15 typically expects a more recent ESLint (>=8.57). You’re on 8.27, which can cause peer warnings or rule resolution issues.
Apply this diff to bump ESLint:
- "eslint": "^8.27.0", + "eslint": "^8.57.0",app/positions/report/[account]/page.tsx (1)
10-12: Avoid Promise-typed params in Page; keep sync if not neededIn App Router, params is a plain object. Typing it as Promise and awaiting adds noise and may confuse type tooling.
Apply this diff:
-export default async function ReportPage({ params }: { params: Promise<{ account: Address }> }) { - const p = await params; - return <ReportContent account={p.account} />; +export default function ReportPage({ params }: { params: { account: Address } }) { + return <ReportContent account={params.account} />;If you intentionally rely on Promise-typed params due to a new Next feature, point me to the docs and I’ll adapt the suggestion.
src/components/SupplyModalContent.tsx (2)
142-170: onPress migration LGTM; minor nits for robustness
- Call switchToNetwork via a function and ignore its Promise to avoid unhandled rejections.
- Use isDisabled (consistent with your Button/NextUI API) instead of disabled for proper styling/variants.
Apply this diff:
- <Button - onPress={switchToNetwork} + <Button + onPress={() => void switchToNetwork()} className="ml-2 min-w-32" variant="secondary" > Switch Chain </Button> @@ - <Button - disabled={!isConnected || isLoadingPermit2 || supplyPending} + <Button + isDisabled={!isConnected || isLoadingPermit2 || supplyPending} onPress={() => void approveAndSupply()} className="ml-2 min-w-32" variant="cta" > Supply </Button> @@ - <Button - disabled={ + <Button + isDisabled={ !isConnected || supplyPending || inputError !== null || !supplyAmount } onPress={() => void signAndSupply()} className="ml-2 min-w-32" variant="cta"
29-33: Fix missing dependency in useCallbackonSuccess uses refetch but it’s not in the deps array; can capture stale ref.
Apply:
- }, [onClose]); + }, [onClose, refetch]);app/global-error.tsx (1)
15-18: Consider not exposing raw error messages to usersRaw error messages can leak internals. Show a generic message in production; keep detail in console.
Example:
{process.env.NODE_ENV === 'development' ? (error?.message ?? 'Unknown error') : null}app/rewards/components/RewardTable.tsx (1)
208-241: onPress event: avoid relying on stopPropagationNextUI onPress typically passes a PressEvent (no stopPropagation). This may be a no-op or TS-incompatible depending on your Button typing. You already prevent row propagation on the Link; consider dropping stopPropagation here.
Apply:
- onPress={(e) => { - void (async () => { - e.stopPropagation(); + onPress={() => { + void (async () => { if (!account) {Also, waiting a fixed 1s after switchToNetwork is brittle. Prefer awaiting the switch if your hook returns a Promise or fire sendTransaction in onNetworkSwitched.
Confirm the onPress event type in src/components/common/Button.tsx and whether switchToNetwork returns a Promise. If yes, replace the timeout with await switchToNetwork().
app/error.tsx (2)
4-4: Verify Barrel export for Button path.This imports from '@/components/common'. Other files import from '@/components/common/Button'. For consistency and to avoid path drift, consider standardizing.
7-10: Optional: capture more error context.If available, also log error.digest to aid debugging in Next.js.
- console.error('App error boundary caught:', error); + console.error('App error boundary caught:', { error, digest: (error as any)?.digest });app/positions/components/onboarding/SuccessPage.tsx (1)
39-41: Grammar nit: “automate reallocate” → “automatically reallocate”.Small copy tweak.
- {allowAgentSetting && 'You can set Monarch Agent to automate reallocate your positions.'} + {allowAgentSetting && 'You can set Monarch Agent to automatically reallocate your positions.'}public/serviceWorker.js (1)
16-16: Drop the empty fetch handler.A no-op fetch listener is unnecessary; remove to keep the SW minimal.
-self.addEventListener('fetch', () => {});src/OnchainProviders.tsx (1)
41-43: Useless Fragment; return children directly.Minor cleanup (also covered by the fix above if you adopt it).
- return <>{children}</>; + return children;app/layout.tsx (1)
26-31: Hydration suppression on html/body — consider scoping it tighterSetting suppressHydrationWarning on both html and body can hide real mismatches. If the mismatch is only due to theme/fonts, prefer limiting suppression to the smallest surface (e.g., theme root or specific nodes).
Can you confirm which elements still mismatch without the global suppression? If it’s only theme class toggling, we can move suppression to the theme wrapper.
src/data-sources/subgraph/fetchers.ts (1)
19-28: GraphQL errors now non-throwing — tighten typing and logsThe behavior change is fine. Two improvements:
- Include apiUrl in the warning for quicker triage.
- Consider a typed GraphQLResponse return in the future to force call sites to handle errors.
Apply this diff for better logging context:
- console.warn('Subgraph API GraphQL Error (non-fatal):', result.errors); + console.warn('Subgraph API GraphQL Error (non-fatal)', { + apiUrl, + errors: (result as any).errors, + });Ensure all callers either inspect
response.errorsor have surrounding guards (as you did in markets/liquidations).src/data-sources/subgraph/liquidations.ts (1)
36-40: Parity with markets filter: include zero addressFor consistency with fetchSubgraphMarkets, also exclude the zero address here.
- where: { inputToken_not_in: blacklistTokens }, + where: { inputToken_not_in: [...blacklistTokens, '0x0000000000000000000000000000000000000000'] },src/data-sources/subgraph/market.ts (2)
305-324: Log GraphQL errors (non-fatal) to aid triageFetcher no longer throws on GraphQL errors. Log them here too for market-specific context.
const response = await subgraphGraphqlFetcher<SubgraphMarketQueryResponse>( subgraphApiUrl, subgraphMarketQuery, { id: uniqueKey.toLowerCase(), }, ); + if (response.errors?.length) { + console.warn('Subgraph GraphQL errors (non-fatal):', response.errors); + }
357-377: Minor: avoid double-cast and log GraphQL errors for context
- The
unknown as Record<string, unknown>cast is unnecessary; a single cast is enough.- Add an optional log for non-fatal GraphQL errors.
- const response = await subgraphGraphqlFetcher<SubgraphMarketsQueryResponse>( + const response = await subgraphGraphqlFetcher<SubgraphMarketsQueryResponse>( subgraphApiUrl, subgraphMarketsQuery, - variables as unknown as Record<string, unknown>, + variables as Record<string, unknown>, ); + if (response.errors?.length) { + console.warn('Subgraph GraphQL errors (non-fatal):', response.errors); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
app/error.tsx(1 hunks)app/global-error.tsx(1 hunks)app/layout.tsx(1 hunks)app/market/[chainId]/[marketid]/content.tsx(1 hunks)app/markets/components/MarketSettingsModal.tsx(2 hunks)app/markets/components/MarketTableBody.tsx(1 hunks)app/markets/components/markets.tsx(2 hunks)app/positions/components/FromAndToMarkets.tsx(1 hunks)app/positions/components/PositionsContent.tsx(3 hunks)app/positions/components/PositionsSummaryTable.tsx(2 hunks)app/positions/components/RebalanceActionInput.tsx(1 hunks)app/positions/components/RebalanceModal.tsx(1 hunks)app/positions/components/SuppliedMarketsDetail.tsx(2 hunks)app/positions/components/onboarding/Modal.tsx(1 hunks)app/positions/components/onboarding/RiskSelection.tsx(1 hunks)app/positions/components/onboarding/SuccessPage.tsx(1 hunks)app/positions/report/[account]/page.tsx(1 hunks)app/rewards/components/RewardTable.tsx(1 hunks)package.json(4 hunks)public/serviceWorker.js(1 hunks)src/OnchainProviders.tsx(1 hunks)src/components/Borrow/AddCollateralAndBorrow.tsx(3 hunks)src/components/Borrow/WithdrawCollateralAndRepay.tsx(2 hunks)src/components/SupplyModalContent.tsx(3 hunks)src/components/WithdrawModalContent.tsx(1 hunks)src/components/layout/header/AccountConnect.tsx(1 hunks)src/components/providers/ThemeProvider.tsx(1 hunks)src/data-sources/subgraph/fetchers.ts(1 hunks)src/data-sources/subgraph/liquidations.ts(1 hunks)src/data-sources/subgraph/market.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Borrow/AddCollateralAndBorrow.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-16T02:01:51.219Z
Learnt from: antoncoding
PR: antoncoding/monarch#97
File: app/positions/components/onboarding/OnboardingContext.tsx:36-43
Timestamp: 2024-12-16T02:01:51.219Z
Learning: In `app/positions/components/onboarding/OnboardingContext.tsx`, the `defaultStep` variable is no longer needed and can be removed.
Applied to files:
app/positions/components/PositionsContent.tsx
📚 Learning: 2024-12-17T10:51:07.277Z
Learnt from: antoncoding
PR: antoncoding/monarch#99
File: src/components/AgentSetupProcessModal.tsx:38-51
Timestamp: 2024-12-17T10:51:07.277Z
Learning: In `src/components/AgentSetupProcessModal.tsx`, the modal is intentionally designed without a close button.
Applied to files:
app/positions/components/PositionsContent.tsxapp/positions/components/onboarding/Modal.tsxapp/positions/components/onboarding/SuccessPage.tsx
📚 Learning: 2024-10-23T16:17:02.841Z
Learnt from: antoncoding
PR: antoncoding/monarch#77
File: src/graphql/queries.ts:168-193
Timestamp: 2024-10-23T16:17:02.841Z
Learning: In `src/graphql/queries.ts`, handling only `MarketTransferTransactionData` is intentional at this time.
Applied to files:
src/data-sources/subgraph/liquidations.tssrc/data-sources/subgraph/market.ts
🧬 Code Graph Analysis (15)
app/global-error.tsx (2)
app/error.tsx (1)
Error(6-21)src/components/common/Button.tsx (1)
Button(4-53)
app/positions/components/PositionsContent.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/positions/components/onboarding/Modal.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
src/components/Borrow/WithdrawCollateralAndRepay.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/markets/components/MarketSettingsModal.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/market/[chainId]/[marketid]/content.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/positions/report/[account]/page.tsx (1)
app/positions/report/components/ReportContent.tsx (1)
ReportContent(34-269)
app/error.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/layout.tsx (1)
app/fonts.ts (3)
zen(3-18)inter(20-35)monospace(37-41)
src/data-sources/subgraph/liquidations.ts (2)
src/data-sources/subgraph/fetchers.ts (1)
subgraphGraphqlFetcher(1-31)src/graphql/morpho-subgraph-queries.ts (1)
subgraphMarketsWithLiquidationCheckQuery(241-258)
src/components/WithdrawModalContent.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/positions/components/onboarding/SuccessPage.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
src/data-sources/subgraph/market.ts (2)
src/data-sources/subgraph/fetchers.ts (1)
subgraphGraphqlFetcher(1-31)src/utils/subgraph-types.ts (2)
SubgraphMarketQueryResponse(86-91)SubgraphMarketsQueryResponse(78-83)
src/OnchainProviders.tsx (1)
src/store/createWagmiConfig.ts (1)
createWagmiConfig(24-67)
src/components/layout/header/AccountConnect.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
🪛 ESLint
app/global-error.tsx
[error] 7-7: elements must have the lang prop.
(jsx-a11y/html-has-lang)
src/OnchainProviders.tsx
[error] 42-42: Fragments should contain more than one child - otherwise, there’s no need for a Fragment at all.
(react/jsx-no-useless-fragment)
🪛 Biome (2.1.2)
app/error.tsx
[error] 6-6: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (24)
app/market/[chainId]/[marketid]/content.tsx (1)
194-201: onPress migration looks goodHandlers map 1:1 from onClick to onPress; no behavior change.
app/positions/components/onboarding/Modal.tsx (1)
84-86: LGTM on onPress switchUsing NextUI’s Button with onPress is consistent with their API.
app/positions/components/RebalanceActionInput.tsx (1)
67-68: LGTM on onPress switchNo event usage; safe migration.
app/positions/components/RebalanceModal.tsx (1)
303-306: LGTM on onPress switchRefresh still does the same thing, just via onPress.
app/markets/components/markets.tsx (1)
432-441: onPress migration looks goodBoth buttons correctly switch to onPress; handlers and accessibility labels remain intact.
Also applies to: 443-452
app/positions/components/PositionsContent.tsx (1)
93-97: Buttons migrated to onPress — OKHandlers are unchanged and semantics preserved for Agent setup, onboarding, and refresh.
Also applies to: 99-107, 162-171
app/positions/components/PositionsSummaryTable.tsx (1)
166-175: Refresh Button: onPress change is correctNo behavior changes; disabled state still respected.
src/components/layout/header/AccountConnect.tsx (1)
44-47: Connect Button: onPress migration is correctMatches the Button API change; handleClicked remains the same.
src/components/WithdrawModalContent.tsx (2)
167-170: onPress migration looks goodSwitch Chain button correctly moved to onPress; no behavior change.
173-173: Async handler usage is safevoid withdraw() avoids unhandled promise warnings and matches onPress signature.
src/components/Borrow/WithdrawCollateralAndRepay.tsx (2)
344-351: onPress handler logic is correctConditional approve vs sign path preserved; void calls prevent promise leakage. isDisabled gating above is consistent.
330-333: Replace unsupportedsolidvariant with a valid oneThe
Buttonwrapper only supports:default,cta,secondary,interactive,ghost. Changevariant="solid"to match other “Switch Chain” buttons:• File:
src/components/Borrow/WithdrawCollateralAndRepay.tsx
Lines ~330- <Button onPress={switchToNetwork} className="min-w-32" variant="solid"> + <Button onPress={switchToNetwork} className="min-w-32" variant="secondary">⛔ Skipped due to learnings
Learnt from: antoncoding PR: antoncoding/monarch#94 File: src/components/supplyModal.tsx:449-452 Timestamp: 2024-12-08T12:10:17.127Z Learning: The 'solid' variant is defined in the new `Button` component and is a valid variant.app/positions/components/SuppliedMarketsDetail.tsx (2)
228-232: onPress migration: Withdraw actionState updates preserved; no regressions.
238-244: onPress migration: Supply actionLooks good; consistent with Button API.
app/markets/components/MarketSettingsModal.tsx (2)
237-246: Preset entries: onPress migration is correctUsing NextUI Button; onPress is the right event. No logic changes.
259-261: Custom entries submit: onPress migration is correctEvent prop updated; handler unchanged. Enter key handling remains intact.
app/positions/components/onboarding/RiskSelection.tsx (1)
207-213: onPress migration for Details button is correctPreserves stopPropagation and new tab open behavior.
package.json (4)
49-49: Next.js 15 upgrade looks goodUpgrade to next@15.1.6 is aligned with the Node bump and TS update.
130-130: Yarn 4 setup OK — node-modules mode confirmed
Your .yarnrc.yml already sets
- nodeLinker: node-modules
- yarnPath: .yarn/releases/yarn-4.1.0.cjs
Since you’re using the classic node_modules linker, no PnP bridges (pnpify) are needed for Jest (you have jest.config.js).
Everything looks configured correctly.
88-88: ts-jest TypeScript compatibility confirmed
ts-jest@29.1.1 declares"typescript": ">=4.3 <6", so your TypeScript ~5.6.3 is fully supported. No further changes needed here.
131-133: Resolution on tr46: no active dependentsYour yarn.lock doesn’t list tr46 or whatwg-url, so the
"resolutions": { "tr46": "^4.0.0" }entry is effectively a no-op today. There are no packages pulling in tr46, so this bump won’t break anything.app/positions/components/onboarding/SuccessPage.tsx (1)
45-52: onPress migration looks good.Button handler migration to onPress matches the updated Button API.
public/serviceWorker.js (1)
1-14: No Service Worker registration detected
I searched for allnavigator.serviceWorker.register(…)calls and found none. This SW file isn’t being re-registered on page load and won’t immediately unregister itself.src/components/providers/ThemeProvider.tsx (1)
15-16: LGTM: suppressHydrationWarning is appropriate here.This helps avoid hydration mismatches with theme class switching.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (9)
src/components/layout/header/AccountDropdown.tsx (1)
64-65: Update DropdownItem handlers: use onPress and harden external linksPlease apply these changes for consistency with the new React Aria API and to mitigate reverse tab-nabbing:
• src/components/layout/header/AccountDropdown.tsx
<DropdownItem key="copy" - onClick={handleCopyAddress} + onPress={handleCopyAddress} endContent={<CopyIcon className="h-4 w-4" />} /> <DropdownItem key="explorer" endContent={<ExternalLinkIcon className="h-4 w-4" />} - onClick={() => window.open(getExplorerURL(address, chainId ?? 1), '_blank')} + onPress={() => + window.open( + getExplorerURL(address, chainId ?? 1), + '_blank', + 'noopener,noreferrer' + ) + } > View on Explorer </DropdownItem> <DropdownItem key="logout" - onClick={handleDisconnectWallet} + onPress={handleDisconnectWallet} endContent={<ExitIcon className="h-4 w-4" />} className="text-red-500 data-[hover=true]:text-red-500" />• src/components/layout/header/Navbar.tsx
<DropdownItem key="docs" endContent={<RiBookLine className="h-4 w-4" />} - onClick={() => window.open(EXTERNAL_LINKS.docs, '_blank')} + onPress={() => + window.open(EXTERNAL_LINKS.docs, '_blank', 'noopener,noreferrer') + } /> … <DropdownItem key="discord" endContent={<RiDiscordFill className="h-4 w-4" />} - onClick={() => window.open(EXTERNAL_LINKS.discord, '_blank')} + onPress={() => + window.open(EXTERNAL_LINKS.discord, '_blank', 'noopener,noreferrer') + } /> … <DropdownItem key="github" endContent={<RiGithubFill className="h-4 w-4" />} - onClick={() => window.open(EXTERNAL_LINKS.github, '_blank')} + onPress={() => + window.open(EXTERNAL_LINKS.github, '_blank', 'noopener,noreferrer') + } /> … <DropdownItem key="theme" endContent={mounted && (theme === 'dark' ? <LuSunMedium size={16}/> : <FaRegMoon size={14}/>)} - onClick={toggleTheme} + onPress={toggleTheme} > {theme === 'dark' ? 'Light Theme' : 'Dark Theme'} </DropdownItem>src/utils/subgraph-urls.ts (1)
29-34: Fix type-safety: omit keys when URL is undefinedWith
Partial<Record<SupportedNetworks, string>>, values must bestringwhen present. Currently you assign potentiallyundefinedvalues to present keys, which violates the type (and can fail under strict TS).Build the object conditionally so keys only exist when the URL exists.
Apply this diff:
-export const SUBGRAPH_URLS: Partial<Record<SupportedNetworks, string>> = { - [SupportedNetworks.Base]: baseSubgraphUrl, - [SupportedNetworks.Mainnet]: mainnetSubgraphUrl, - [SupportedNetworks.Polygon]: polygonSubgraphUrl, - [SupportedNetworks.Unichain]: unichainSubgraphUrl, -}; +export const SUBGRAPH_URLS: Partial<Record<SupportedNetworks, string>> = { + ...(baseSubgraphUrl ? { [SupportedNetworks.Base]: baseSubgraphUrl } : {}), + ...(mainnetSubgraphUrl ? { [SupportedNetworks.Mainnet]: mainnetSubgraphUrl } : {}), + ...(polygonSubgraphUrl ? { [SupportedNetworks.Polygon]: polygonSubgraphUrl } : {}), + ...(unichainSubgraphUrl ? { [SupportedNetworks.Unichain]: unichainSubgraphUrl } : {}), +};src/data-sources/subgraph/transactions.ts (1)
117-124: Bug: undefined marketUniqueKeys yields empty resultsWhen
filters.marketUniqueKeysis undefined, the current predicate evaluates to undefined (falsy) and filters out everything. Expected behavior (per comment) is “all markets.”Apply this diff:
- // marketUniqueKeys is empty: all markets - const filteredTransactions = - filters.marketUniqueKeys?.length === 0 - ? allTransactions - : allTransactions.filter( - (tx) => filters.marketUniqueKeys?.includes(tx.data.market.uniqueKey), - ); + // marketUniqueKeys absent or empty: all markets + const marketKeys = filters.marketUniqueKeys; + const filteredTransactions = + !marketKeys || marketKeys.length === 0 + ? allTransactions + : allTransactions.filter((tx) => marketKeys.includes(tx.data.market.uniqueKey));src/config/oracle-whitelist.ts (1)
18-22: Trim trailing spaces in addressesTrailing spaces in feed addresses will cause mismatches downstream and make links/copypasta error-prone.
Apply this diff:
- address: '0x3Ea1eC855fBda8bA0396975eC260AD2e9B2Bc01c ', + address: '0x3Ea1eC855fBda8bA0396975eC260AD2e9B2Bc01c', ... - address: '0xaca1222008C6Ea624522163174F80E6e17B0709A ', + address: '0xaca1222008C6Ea624522163174F80E6e17B0709A',Also applies to: 32-36
app/positions/report/components/ReportContent.tsx (5)
45-48: Bug: set() result ignored; twoMonthsAgo doesn’t zero minute/second@internationalized/date objects are immutable; set() returns a new instance. The current code discards it.
- const twoMonthsAgo = useMemo(() => { - const date = now(getLocalTimeZone()).subtract({ months: 2 }); - date.set({ minute: 0, second: 0 }); - return date; - }, []); + const twoMonthsAgo = useMemo(() => { + return now(getLocalTimeZone()).subtract({ months: 2 }).set({ minute: 0, second: 0 }); + }, []);
96-103: Bug: comparing ZonedDateTime with < and >; use compare()ZonedDateTime doesn’t support JS relational operators. Use compare() like elsewhere in this file.
const handleStartDateChange = (date: ZonedDateTime) => { // Ensure time is set to exact hour const exactHourDate = date.set({ minute: 0, second: 0 }); - if (exactHourDate > endDate) { + if (exactHourDate.compare(endDate) > 0) { setEndDate(exactHourDate); } setStartDate(exactHourDate); }; const handleEndDateChange = (date: ZonedDateTime) => { // Ensure time is set to exact hour const exactHourDate = date.set({ minute: 0, second: 0 }); - if (exactHourDate < startDate) { + if (exactHourDate.compare(startDate) < 0) { setStartDate(exactHourDate); } setEndDate(exactHourDate); };Also applies to: 105-112
82-86: Use value equality for date changes; avoid referential checksThese are new instances; !== will often be true even for equal moments. Use compare() like in isReportCurrent.
- reportState.asset.address !== selectedAsset.address || - reportState.startDate !== startDate || - reportState.endDate !== endDate + reportState.asset.address !== selectedAsset.address || + reportState.startDate.compare(startDate) !== 0 || + reportState.endDate.compare(endDate) !== 0
169-181: Guard against positions being undefined in useMemopositions may be undefined while loading; positions.forEach(...) will throw before the loading branch renders.
- positions.forEach((position) => { + (positions ?? []).forEach((position) => { const key = `${position.market.loanAsset.address}-${position.market.morphoBlue.chain.id}`; if (!assetMap.has(key)) { assetMap.set(key, { symbol: position.market.loanAsset.symbol, address: position.market.loanAsset.address as Address, chainId: position.market.morphoBlue.chain.id, decimals: position.market.loanAsset.decimals, }); } });
239-241: Use onPress, not onClick, with our Button wrapperFor consistency with the Button API migration and NextUI semantics.
- <Button - onClick={() => { + <Button + onPress={() => { void handleGenerateReport(); }}
♻️ Duplicate comments (1)
src/OnchainProviders.tsx (1)
39-48: Fix confirmed: WagmiProvider no longer renders with undefined configReturning children when projectId is missing and lazily caching wagmiConfig addresses the earlier runtime issue.
🧹 Nitpick comments (16)
src/components/layout/header/AccountDropdown.tsx (1)
23-29: Handle clipboard failures.Navigator clipboard can reject (permissions, HTTP). Surface an error toast.
const handleCopyAddress = useCallback(() => { if (address) { - void navigator.clipboard.writeText(address).then(() => { - toast.success('Address copied', 'Address copied to clipboard'); - }); + void navigator.clipboard + .writeText(address) + .then(() => { + toast.success('Address copied', 'Address copied to clipboard'); + }) + .catch(() => { + toast.error('Copy failed', 'Unable to write to clipboard'); + }); } }, [address, toast]);src/data-sources/subgraph/market.ts (2)
300-303: Consider returning safe fallbacks instead of throwing on missing URLElsewhere (historical, transactions) we degrade gracefully when the subgraph URL is absent. Throwing here can bubble and break SSR.
Apply this diff:
if (!subgraphApiUrl) { console.error(`Subgraph URL for network ${network} is not defined.`); - throw new Error(`Subgraph URL for network ${network} is not defined.`); + return null; }And similarly for the list fetcher:
if (!subgraphApiUrl) { console.error(`Subgraph URL for network ${network} is not defined.`); - throw new Error(`Subgraph URL for network ${network} is not defined.`); + return []; }Also applies to: 345-348
112-116: Remove leftover debug logThis targeted log will spam the console in prod.
Apply this diff:
- if ( - marketId.toLowerCase() === '0x9103c3b4e834476c9a62ea009ba2c884ee42e94e6e314a26f04d312434191836' - ) { - console.log('subgraphMarket', subgraphMarket); - }src/config/oracle-whitelist.ts (1)
116-121: Normalize oracle address lookup to avoid case mismatchesWhitelist keys are lowercase; incoming addresses may be checksum/mixed-case. Normalize input to ensure consistent hits.
Apply this diff:
export const getWhitelistedOracleData = ( oracleAddress: Address, network: SupportedNetworks, ): WhitelistedOracleData | undefined => { - return oracleWhitelist[network]?.[oracleAddress]; + const list = oracleWhitelist[network]; + if (!list) return undefined; + return list[oracleAddress] ?? list[oracleAddress.toLowerCase() as Address]; };app/positions/report/components/ReportContent.tsx (1)
212-214: Avoid any-casts for DatePicker value; use DateValue typingZonedDateTime is a DateValue; prefer a typed cast to DateValue over any. This keeps type-safety and removes the lint disable.
- // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - value={startDate as any} + value={startDate as DateValue} ... - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - value={endDate as any} + value={endDate as DateValue}Follow-up: consider updating DatePicker’s props to generically accept ZonedDateTime so you can also drop the
as anyon onChange.Also applies to: 225-227
app/history/[account]/page.tsx (1)
9-11: Don’t typeparamsas a Promise;awaitis unnecessary for page propsIn App Router,
paramsis a plain object. Typing it as a Promise isn’t aligned with Next.js and can confuse TS tooling.await paramswon’t break at runtime but hurts DX.Proposed fix:
-export default async function HistoryPage({ params }: { params: Promise<{ account: string }> }) { - const p = await params; - return <HistoryContent account={p.account} />; +export default async function HistoryPage({ params }: { params: { account: string } }) { + return <HistoryContent account={params.account} />; }Please double-check Next.js 15 docs for page component props to confirm
paramsisn’t a Promise.package.json (1)
130-134: Add engines.node to enforce Node 22; verify if tr46/punycode resolutions are still needed
- Add an
enginesfield so CI and package managers enforce the Node bump.- The
resolutionsfor tr46/punycode might be redundant with the upgraded tree; keep if needed, but worth verifying to reduce lockfile churn.Suggested additions (outside this hunk):
"private": true, + "engines": { + "node": ">=22.12.0" + },If you keep the resolutions, consider documenting the reason in the PR or a comment to help future maintainers.
Can you confirm whether these resolutions address a specific issue (e.g., URL/IDNA behavior under Node 22) and are still necessary after the Next 15 upgrade?
app/rewards/components/RewardTable.tsx (1)
208-239: Avoid race with setState before switching network; await actual switch instead of a fixed delayYou set
targetChainId, then immediately callswitchToNetwork()which likely reads the previous state. The 1s timeout is brittle.Prefer passing the target chain directly (if supported) and awaiting the switch. Example:
- if (chainId !== distribution.distributor.chain_id) { - // Set the target chain ID and switch - setTargetChainId(distribution.distributor.chain_id); - switchToNetwork(); - // Wait for network switch - await new Promise((resolve) => setTimeout(resolve, 1000)); - } + if (chainId !== distribution.distributor.chain_id) { + // Switch directly to the required chain and await completion + await switchToNetwork(distribution.distributor.chain_id); + }If
switchToNetworkdoesn’t accept a parameter, consider adjusting the hook to:
- accept a
chainIdargument, or- return a Promise resolved via its
onNetworkSwitchedcallback, so you canawaitit here instead ofsetTimeout.This will remove potential flakiness when claiming on a different network.
Please confirm the signature of
useMarketNetwork().switchToNetworkand whether it can be updated to accept a target chain ID or return a Promise.src/hooks/useLocalStorage.ts (1)
26-37: Post-mount hydration is correct; consider dropping local-storage-fallbackThe rehydrate-after-mount pattern avoids SSR mismatches. Given the new SSR guards, you can likely remove the
local-storage-fallbackdependency and usewindow.localStoragedirectly whentypeof window !== 'undefined'. This reduces deps and makes intent clear. I can draft a small refactor if you want.Happy to push a minimal refactor to native localStorage if desired.
app/markets/components/markets.tsx (1)
271-287: Tighten sort column validation; optionally reset direction when switching columnsObject.values on numeric enums includes both numbers and strings; filter to numbers to avoid false positives. Also consider resetting direction when changing columns (keeps UX predictable).
Apply within the callback:
const titleOnclick = useCallback( (column: number) => { - // Validate that column is a valid SortColumn value - const isValidColumn = Object.values(SortColumn).includes(column); - if (!isValidColumn) { + // Validate numeric enum values only + const isValidColumn = (Object.values(SortColumn) as unknown[]).some( + (v) => typeof v === 'number' && v === column, + ); + if (!isValidColumn) { console.error(`Invalid sort column value: ${column}`); return; } - setSortColumn(column); - - if (column === sortColumn) { - setSortDirection(-sortDirection); - } + if (column === sortColumn) { + setSortDirection((d) => -d); + } else { + setSortColumn(column); + // Optional: reset direction on new column + setSortDirection(-1); + } }, [sortColumn, sortDirection, setSortColumn, setSortDirection], );If you prefer avoiding recomputing valid values per call, define a Set once:
// Place inside component, above titleOnclick const validSortColumns = useMemo( () => new Set<number>((Object.values(SortColumn) as unknown[]).filter((v): v is number => typeof v === 'number')), [], );Then replace the validation with: if (!validSortColumns.has(column)) …
src/OnchainProviders.tsx (2)
12-32: Harden retry logic: guard error type before reading messageerror may be non-Error (string/unknown). Guarding avoids edge cases and type noise.
defaultOptions: { queries: { - retry: (failureCount, error) => { - // Don't retry on GraphQL errors, network errors, or client errors - if (error?.message?.includes('GraphQL') || error?.message?.includes('Network response was not ok')) { - return false; - } - // Retry up to 2 times for other errors - return failureCount < 2; - }, + retry: (failureCount, error) => { + // Normalize error message safely + const msg = + typeof error === 'string' + ? error + : (error && typeof error === 'object' && 'message' in error && typeof (error as any).message === 'string' + ? (error as any).message + : ''); + // Don't retry on GraphQL errors or failed fetch responses + if (msg.includes('GraphQL') || msg.includes('Network response was not ok')) { + return false; + } + // Retry up to 2 times for other errors + return failureCount < 2; + }, retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000), // Prevent queries from throwing and crashing the app throwOnError: false, },
37-37: Remove stray ESLint disableThis directive targets promise-function-async but the following function isn’t async. Safe to drop.
-// eslint-disable-next-line @typescript-eslint/promise-function-async.eslintrc.js (4)
43-45: Consider keeping array-as-prop rule at least as a warningNew arrays in props often cause needless re-renders. A warn level keeps signal without blocking.
- 'react-perf/jsx-no-new-array-as-prop': 'off', + 'react-perf/jsx-no-new-array-as-prop': 'warn',
73-73: Turning off no-explicit-any globally can mask bugsIf you need flexibility, prefer 'warn' and allow exceptions via inline ts-expect-error.
- '@typescript-eslint/no-explicit-any': 'off', + '@typescript-eslint/no-explicit-any': ['warn', { fixToUnknown: false }],
123-126: Prefer guiding nullish-coalescing and optional chaining rather than disablingThese rules help readability and safety. Downgrade to warn to keep nudge without friction.
- '@typescript-eslint/prefer-nullish-coalescing': 'off', + '@typescript-eslint/prefer-nullish-coalescing': 'warn', - '@typescript-eslint/prefer-optional-chain': 'off', + '@typescript-eslint/prefer-optional-chain': 'warn',
136-138: Avoid disabling exhaustive-deps globallyThis hides real bugs. Keep it on (or warn) and suppress locally where intentional.
- // Hook dependency warnings are intentionally skipped to ensure flow - 'react-hooks/exhaustive-deps': 'off', + // Prefer local disables where necessary + 'react-hooks/exhaustive-deps': 'warn',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.claude/settings.local.json(1 hunks).eslintrc.js(4 hunks).yarnrc.yml(1 hunks)app/error.tsx(1 hunks)app/global-error.tsx(1 hunks)app/history/[account]/page.tsx(1 hunks)app/markets/components/markets.tsx(4 hunks)app/markets/components/utils.ts(1 hunks)app/markets/page.tsx(2 hunks)app/positions/components/FromAndToMarkets.tsx(1 hunks)app/positions/components/onboarding/RiskSelection.tsx(2 hunks)app/positions/report/components/ReportContent.tsx(2 hunks)app/rewards/components/RewardTable.tsx(1 hunks)package.json(4 hunks)src/OnchainProviders.tsx(1 hunks)src/components/WithdrawModalContent.tsx(1 hunks)src/components/layout/header/AccountDropdown.tsx(1 hunks)src/config/oracle-whitelist.ts(1 hunks)src/data-sources/subgraph/historical.ts(1 hunks)src/data-sources/subgraph/market.ts(2 hunks)src/data-sources/subgraph/transactions.ts(2 hunks)src/hooks/useLocalStorage.ts(3 hunks)src/hooks/useUserPositionsSummaryData.ts(1 hunks)src/utils/gtag.ts(0 hunks)src/utils/subgraph-urls.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/utils/gtag.ts
✅ Files skipped from review due to trivial changes (2)
- app/markets/components/utils.ts
- src/hooks/useUserPositionsSummaryData.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/global-error.tsx
- app/positions/components/FromAndToMarkets.tsx
- app/error.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-23T16:17:02.841Z
Learnt from: antoncoding
PR: antoncoding/monarch#77
File: src/graphql/queries.ts:168-193
Timestamp: 2024-10-23T16:17:02.841Z
Learning: In `src/graphql/queries.ts`, handling only `MarketTransferTransactionData` is intentional at this time.
Applied to files:
src/data-sources/subgraph/transactions.tssrc/data-sources/subgraph/market.ts
📚 Learning: 2024-10-12T09:23:16.495Z
Learnt from: antoncoding
PR: antoncoding/monarch#63
File: app/markets/components/MarketRowDetail.tsx:49-52
Timestamp: 2024-10-12T09:23:16.495Z
Learning: When rendering oracle feeds in `ExpandedMarketDetail` (`app/markets/components/MarketRowDetail.tsx`), prefer explicit rendering over iterating keys when dealing with a small number of feeds.
Applied to files:
app/positions/components/onboarding/RiskSelection.tsx
🧬 Code Graph Analysis (9)
app/history/[account]/page.tsx (1)
app/history/components/HistoryContent.tsx (1)
HistoryContent(8-25)
src/data-sources/subgraph/transactions.ts (3)
src/data-sources/subgraph/types.ts (1)
SubgraphAccountData(51-57)src/hooks/useUserTransactions.ts (2)
TransactionFilters(8-18)TransactionResponse(20-27)src/utils/types.ts (1)
UserTransaction(29-41)
src/config/oracle-whitelist.ts (1)
src/utils/networks.ts (1)
SupportedNetworks(52-52)
src/utils/subgraph-urls.ts (1)
src/utils/networks.ts (1)
SupportedNetworks(52-52)
app/markets/components/markets.tsx (1)
src/hooks/useLocalStorage.ts (1)
useLocalStorage(4-78)
app/positions/components/onboarding/RiskSelection.tsx (1)
src/utils/types.ts (1)
Market(262-310)
src/OnchainProviders.tsx (1)
src/store/createWagmiConfig.ts (1)
createWagmiConfig(24-67)
src/components/WithdrawModalContent.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
src/data-sources/subgraph/market.ts (2)
src/data-sources/subgraph/fetchers.ts (1)
subgraphGraphqlFetcher(1-31)src/utils/subgraph-types.ts (2)
SubgraphMarketQueryResponse(86-91)SubgraphMarketsQueryResponse(78-83)
🔇 Additional comments (17)
src/components/layout/header/AccountDropdown.tsx (1)
56-56: Good call adding a stable key.NextUI’s DropdownMenu items expect unique keys; this avoids React warnings and selection glitches.
src/utils/subgraph-urls.ts (1)
29-29: Type migration looks goodSwitch to
Partial<Record<...>>is idiomatic and consistent with the rest of the PR.src/data-sources/subgraph/historical.ts (1)
135-143: Good defensive check with optional chainingUsing
!response?.dataaligns with the fetcher’s non-throwing behavior and prevents crashes on partial/missing payloads.src/data-sources/subgraph/transactions.ts (2)
16-29: Graceful handling for null/undefined subgraph dataEarly-return keeps the transformer robust and avoids branching later. Nice.
212-213: Call-site update is correctPassing
result?.data?.accountmatches the transformer’s new nullable input and avoids exceptions.src/data-sources/subgraph/market.ts (2)
305-325: try/catch around subgraph fetch: goodCatches network/parse errors and returns
nullwith context logs. The follow-up checks for missingmarketare tidy.
358-382: Graceful handling for markets list: goodValidates data shape, logs a concise warning, fetches price hints once, and maps. Solid.
src/config/oracle-whitelist.ts (2)
14-14: Type migration to Partial<Record<...>> looks goodMatches the pattern used across the PR and keeps network keys optional.
88-95: Verify feed id for WBTC/USD entryThe
idis set to the zero address whileaddressis non-zero. Likely a copy/paste error.Do you want me to fix the
idto0xaca1222008C6Ea624522163174F80E6e17B0709Ato match the address (or another correct feed id)?.yarnrc.yml (1)
3-3: LGTM: explicit npm registrySetting npmRegistryServer to the npm registry is fine and consistent with Yarn 4 + nodeLinker=node-modules usage.
app/markets/page.tsx (1)
14-18: LGTM: Suspense boundary addedWrapping MarketContent with Suspense and a simple fallback is a safe improvement.
src/components/WithdrawModalContent.tsx (1)
167-174: LGTM: onPress + isDisabled adoptionThanks for aligning with the Button API (onPress, isDisabled). Matches prior guidance and wrapper behavior.
package.json (1)
49-49: Tooling upgrades look consistentNext.js 15.4.7 + TS 5.6 + @types/node 22 + eslint-config-next 15.x aligns well. No blockers spotted here.
Also applies to: 88-88, 101-101, 128-128
app/positions/components/onboarding/RiskSelection.tsx (1)
206-213: onPress migration looks goodThe Button’s onPress update and handler signature change are consistent with the new Button API.
src/hooks/useLocalStorage.ts (1)
11-15: SSR guards look goodEarly return of
initialValueon SSR and guarding the storage event listener prevent server-side crashes. Nice.Also applies to: 59-60
app/markets/components/markets.tsx (2)
66-67: Good move: persist sort state via SSR-safe useLocalStorageThis aligns with the repo’s SSR patterns and avoids hydration issues while keeping cross-tab sync.
418-427: Button event prop migration looks consistentonPress matches the Button API change and avoids mixing onClick and onPress in the same view.
Also applies to: 429-436
| const handleMarketDetails = (market: Market) => { | ||
| const marketPath = `/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`; | ||
| window.open(marketPath, '_blank'); | ||
| }; |
There was a problem hiding this comment.
Fix reverse tabnabbing when using window.open
Opening a new tab without noopener allows the new page to access window.opener. Use noopener,noreferrer and null out the opener.
- const handleMarketDetails = (market: Market) => {
- const marketPath = `/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`;
- window.open(marketPath, '_blank');
- };
+ const handleMarketDetails = (market: Market) => {
+ const marketPath = `/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`;
+ const newWin = window.open(marketPath, '_blank', 'noopener,noreferrer');
+ if (newWin) newWin.opener = null;
+ };🤖 Prompt for AI Agents
In app/positions/components/onboarding/RiskSelection.tsx around lines 89 to 92,
the call to window.open() with target '_blank' is vulnerable to reverse
tabnabbing; change it to open with noopener and noreferrer and then explicitly
null out the opener on the returned window. Concretely, call window.open with a
thirdFeatures string that includes "noopener,noreferrer" (or use an anchor with
rel="noopener noreferrer") and assign the result to a variable, then if that
variable is non-null set its opener property to null to fully prevent access to
window.opener.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (15)
src/components/FeedInfo/OracleFeedInfo.tsx (1)
48-55: Add rel="noopener noreferrer" to external linkAvoid reverse tabnabbing on target="_blank" links.
Apply:
<Link className="group flex w-full items-center gap-1 text-right no-underline hover:underline" href={getExplorerURL(feed.address as Address, chainId)} - target="_blank" + target="_blank" + rel="noopener noreferrer" >app/markets/components/MarketRowDetail.tsx (1)
30-38: Harden external linkAdd rel to the anchor opening the block explorer.
Apply:
<a className="group flex items-center gap-1 no-underline hover:underline" href={getExplorerURL(market.oracleAddress, market.morphoBlue.chain.id)} - target="_blank" + target="_blank" + rel="noopener noreferrer" >app/positions/components/agent/SetupAgent.tsx (3)
139-144: BigInt comparison bug: comparing bigint to numbercap.amount is bigint (you pass maxUint256 elsewhere). Comparing to 0 (number) will error in TS/JS. Use 0n for BigInt.
- pendingCaps.some((cap) => cap.market.uniqueKey === market.uniqueKey && cap.amount > 0); + pendingCaps.some((cap) => cap.market.uniqueKey === market.uniqueKey && cap.amount > 0n);
345-351: Modal rendered inside map() creates multiple instancesWhen showProcessModal is true, AgentSetupProcessModal is rendered once per group. That can cause duplicated portals/overlays and accessibility issues. Render it once outside the map.
Remove the modal from inside the map:
- {showProcessModal && ( - <AgentSetupProcessModal - currentStep={currentStep} - onClose={() => setShowProcessModal(false)} - /> - )}Then render it once near the bottom (e.g., just before the final action buttons):
{showProcessModal && ( <AgentSetupProcessModal currentStep={currentStep} onClose={() => setShowProcessModal(false)} /> )}
461-466: Use onPress with the shared Button componentThe project migrated Button to onPress. This onClick likely won’t fire.
- <Button - variant="light" - size="sm" - onClick={() => setShowAllMarkets(true)} - className="w-full" - > + <Button + variant="light" + size="sm" + onPress={() => setShowAllMarkets(true)} + className="w-full" + >app/history/components/HistoryTable.tsx (1)
410-417: Add rel="noopener noreferrer" for external linkExternal target="_blank" without rel can allow tabnabbing.
- <Link + <Link href={getExplorerTxURL(tx.hash, market.morphoBlue.chain.id)} target="_blank" + rel="noopener noreferrer" className="flex items-center gap-1 text-sm hover:underline" >src/components/layout/header/AccountDropdown.tsx (1)
62-82: Use onPress (or menu onAction) instead of onClick for DropdownItemHeroUI/NextUI v2 components are press-event based; onClick can miss keyboard/touch interactions.
Apply this diff:
- <DropdownItem - key="copy" - onClick={handleCopyAddress} + <DropdownItem + key="copy" + onPress={handleCopyAddress} endContent={<CopyIcon className="h-4 w-4" />} > Copy Address </DropdownItem> <DropdownItem key="explorer" endContent={<ExternalLinkIcon className="h-4 w-4" />} - onClick={() => window.open(getExplorerURL(address, chainId ?? 1), '_blank')} + onPress={() => { + const url = getExplorerURL(address, chainId ?? 1); + const w = window.open(url, '_blank', 'noopener,noreferrer'); + if (w) w.opener = null; + }} > View on Explorer </DropdownItem> <DropdownItem key="logout" - onClick={handleDisconnectWallet} + onPress={handleDisconnectWallet} endContent={<ExitIcon className="h-4 w-4" />} className="text-red-500 data-[hover=true]:text-red-500" > Log out </DropdownItem>app/positions/components/PositionsSummaryTable.tsx (2)
166-175: Button disable prop should be 'isDisabled' (Heroui), not 'disabled'Using 'disabled' on the Heroui-based Button wrapper likely has no effect.
Apply:
- <Button + <Button variant="light" size="sm" onPress={handleManualRefresh} - disabled={isRefetching} + isDisabled={isRefetching} className="font-zen text-secondary opacity-80 transition-all duration-200 ease-in-out hover:opacity-100" >
366-385: Rebalance Button: use 'isDisabled' instead of 'disabled'Aligns with Heroui prop naming; prevents the button from staying clickable for non-owners.
<Button size="sm" variant="interactive" className="text-xs" onPress={() => { if (!isOwner) { toast.error( 'No authorization', 'You can only rebalance your own positions', ); return; } setSelectedGroupedPosition(groupedPosition); setShowRebalanceModal(true); }} - disabled={!isOwner} + isDisabled={!isOwner} > Rebalance </Button>package.json (1)
5-19: Scripts still use yarn while packageManager is pnpm — update to pnpm.These will fail in CI/dev if yarn isn’t present.
Apply:
"scripts": { - "build": "rm -rf .next && next build", - "check": "yarn format && yarn lint && yarn stylelint", + "build": "rm -rf .next && next build", + "check": "pnpm run format && pnpm run lint && pnpm run stylelint", "dev": "next dev", "format": "prettier --log-level warn --write .", "format:check": "prettier --check .", "lint": "next lint --fix", "lint:check": "next lint", "start": "next start", "stylelint": "stylelint '**/*.css' --fix", "stylelint:check": "stylelint '**/*.css'", "test": "jest .", "test:coverage": "jest . --coverage", - "test:coverage:open": "yarn test:coverage && open coverage/lcov-report/index.html" + "test:coverage:open": "pnpm run test:coverage && open coverage/lcov-report/index.html" },app/positions/report/components/ReportContent.tsx (1)
45-48: Bug: ZonedDateTime.set() is immutable; current code discards the resultThis returns the unadjusted value (minutes/seconds not zeroed). Use the returned instance.
- const twoMonthsAgo = useMemo(() => { - const date = now(getLocalTimeZone()).subtract({ months: 2 }); - date.set({ minute: 0, second: 0 }); - return date; - }, []); + const twoMonthsAgo = useMemo( + () => + now(getLocalTimeZone()) + .subtract({ months: 2 }) + .set({ minute: 0, second: 0 }), + [], + );app/positions/components/onboarding/SetupPositions.tsx (3)
76-94: Clear error state on valid input (Execute can stay disabled otherwise)If the user fixes an invalid amount, error isn’t cleared. This keeps the Execute button disabled even after valid input.
Apply this diff:
try { // Validate the new amount can be converted to BigInt parseUnits(cleanValue || '0', tokenDecimals); + setError(null); setTotalAmount(cleanValue); } catch (e) { setError('Invalid amount'); }
225-234: Await network switch; avoid brittle fixed sleepNot awaiting
switchToNetwork()and sleeping 1s is racy. If it returns a Promise (most chain switchers do), await it and drop the sleep.- if (needSwitchChain && selectedToken) { - try { - switchToNetwork(); - // Wait for network switch to complete - await new Promise((resolve) => setTimeout(resolve, 1000)); - } catch (switchError) { - toast.error('Failed to switch network', 'Please try again'); - return; - } - } + if (needSwitchChain && selectedToken) { + try { + // If switchToNetwork returns a promise, await it; otherwise Promise.resolve is a no-op. + await Promise.resolve(switchToNetwork()); + } catch (switchError) { + toast.error('Failed to switch network', 'Please try again'); + return; + } + }
275-282: Guard next/image src; empty string will throw
next/imageerrors on empty src. Provide a fallback whenlogoURIis missing.- <Image - src={selectedToken.logoURI ?? ''} - alt={selectedToken.symbol} - width={20} - height={20} - className="rounded-full" - /> + {selectedToken.logoURI ? ( + <Image + src={selectedToken.logoURI} + alt={selectedToken.symbol} + width={20} + height={20} + className="rounded-full" + /> + ) : ( + <div className="flex h-5 w-5 items-center justify-center rounded-full bg-gray-300 text-[10px]"> + {selectedToken.symbol?.slice(0, 2)?.toUpperCase() ?? '?'} + </div> + )}src/components/Borrow/AddCollateralAndBorrow.tsx (1)
86-92: BigInt math: avoidBigInt(10 ** N); use BigInt literalsUsing
BigInt(10 ** N)relies on Number intermediates and can introduce precision issues. Use10n ** Nnto keep everything in BigInt space.- (BigInt(currentPosition.state.collateral) * oraclePrice) / BigInt(10 ** 36); + (BigInt(currentPosition.state.collateral) * oraclePrice) / (10n ** 36n); - const ltv = (currentBorrowValue * BigInt(10 ** 18)) / currentCollateralValue; + const ltv = (currentBorrowValue * (10n ** 18n)) / currentCollateralValue; - const newCollateralValueInLoan = (newCollateral * oraclePrice) / BigInt(10 ** 36); + const newCollateralValueInLoan = (newCollateral * oraclePrice) / (10n ** 36n); - const ltv = (newBorrow * BigInt(10 ** 18)) / newCollateralValueInLoan; + const ltv = (newBorrow * (10n ** 18n)) / newCollateralValueInLoan;Also applies to: 104-109
♻️ Duplicate comments (1)
.claude/settings.local.json (1)
5-5: Replace invalid Next lint allow-list entry with concrete commands"Bash(npx next lint:*)" isn’t a valid command pattern for this permissions schema. Allow explicit invocations instead.
- "Bash(npx next lint:*)", + "Bash(npx next lint)", + "Bash(npx next lint --fix)",
🧹 Nitpick comments (40)
.claude/settings.local.json (3)
19-19: Confirm the web fetch domain scope is sufficientIf the UI migration needs docs or assets from non-www subdomains (e.g., heroui.com root, cdn), you may need additional entries. If www.heroui.com is all you need, this is fine.
1-23: Did you intend to commit a “local” settings file?The filename suggests developer-local config. If this wasn’t meant to be versioned, add it to .gitignore and commit a settings.local.example.json instead.
4-18: Scope allow-list to only the commands you actually runYour current
.claude/settings.local.jsonis very broad and includes unused tools (pnpm, find, generic node, git checkout, etc.) and wildcards. Let’s tighten it to exactly the scripts in yourpackage.jsonplus any other CLI checks you need:Recommended
permissions.allowblock:"permissions": { "allow": [ "Bash(yarn install)", "Bash(yarn dev)", "Bash(yarn build)", "Bash(NEXT_TELEMETRY_DISABLED=1 yarn build)", "Bash(yarn check)", "Bash(yarn format)", "Bash(yarn format:check)", "Bash(yarn lint)", "Bash(yarn lint:check)", "Bash(yarn stylelint)", "Bash(yarn stylelint:check)", "Bash(yarn test)", "Bash(yarn test:coverage)", "Bash(yarn test:coverage:open)", "Bash(yarn start)", "Bash(node -v)", "Bash(node --version)" ] }• Removed all
:*patterns to enforce least privilege
• Dropped pnpm, find, git checkout, genericnode:*, npm run wildcardsOptional helper to regenerate from your scripts:
jq -r '.scripts | keys[] | "Bash(yarn \(.))"' package.jsonapp/positions/components/onboarding/AssetSelection.tsx (3)
110-115: Nit: add type="button" and drop redundant role="button"Prevents unintended form submission if nested in a form; native button already has the correct role.
- <motion.button - aria-label={`Select ${token.symbol} on ${getNetworkName(token.network)}`} - role="button" + <motion.button + type="button" + aria-label={`Select ${token.symbol} on ${getNetworkName(token.network)}`} key={`${token.symbol}-${token.network}-${idx}`} onClick={() => handleTokenSelect(token)} className="group relative flex items-start gap-4 rounded border border-gray-200 bg-white p-4 text-left transition-all duration-300 hover:border-primary dark:border-gray-700 dark:bg-gray-800/50 dark:hover:bg-gray-800"
101-106: Avoid nested interactive elements (Button inside Link)This renders a button inside an anchor in the DOM. Prefer a polymorphic Button that renders as a Link.
- <Link href="/markets"> - <Button color="primary" className="rounded"> - View Markets - </Button> - </Link> + <Button as={Link} href="/markets" color="primary" className="rounded"> + View Markets + </Button>If your Button doesn’t support
as/href, consider making a styled anchor or updating the Button to be polymorphic.
113-113: Prefer a stable key (address + network) over indexImproves reconciliation stability.
- key={`${token.symbol}-${token.network}-${idx}`} + key={`${token.address}-${token.network}`}app/markets/components/NetworkFilter.tsx (1)
18-24: Prefer onSelectionChange over onChange for HeroUI/NextUI SelectRelying on e.target.value is brittle here; the Select is not a native select. Use onSelectionChange and derive the single key. This also avoids SSR/hydration surprises.
Apply this diff:
- onChange={(e) => { - if (!e.target.value) setSelectedNetwork(null); - const newId = Number(e.target.value); - if (newId && isSupportedChain(newId)) { - setSelectedNetwork(newId); - } - }} + onSelectionChange={(keys) => { + const key = + keys === 'all' + ? null + : Array.from(keys as Set<string>)[0] ?? null; + if (!key) { + setSelectedNetwork(null); + return; + } + const newId = Number(key); + if (isSupportedChain(newId)) { + setSelectedNetwork(newId); + } + }}src/components/FeedInfo/OracleFeedInfo.tsx (1)
44-47: Tooltip styling: prefer classNames over classNameHeroUI/NextUI Tooltips typically style the popover via classNames.content. Using className may be ignored.
Proposed change:
<Tooltip content={feed.description ?? getSlicedAddress(feed.address as Address)} - className="rounded-sm" + classNames={{ content: 'rounded-sm' }} >app/markets/components/MarketRowDetail.tsx (1)
43-55: Tooltip popover styling: use classNames.contentEnsures the max-width/rounded/padding apply to the tooltip panel across libraries.
Apply:
- <Tooltip - content={ + <Tooltip + content={ <TooltipContent icon={<QuestionMarkCircledIcon className="h-4 w-4" />} title="Feed Routes" detail="Feed routes show how asset prices are derived from different oracle providers" /> - } - className="max-w-[400px] rounded-sm p-2" - > + } + classNames={{ content: 'max-w-[400px] rounded-sm p-2' }} + >app/admin/stats/page.tsx (1)
53-74: Noise from debug logsConsider gating console logs with an env flag or removing to keep the UI quiet in production.
Example:
- console.log( + if (process.env.NEXT_PUBLIC_DEBUG === '1') console.log( `Fetching statistics for timeframe: ${timeframe}, network: ${ getNetworkName(selectedNetwork) ?? 'Unknown' }`, ); - const startTime = performance.now(); + const startTime = process.env.NEXT_PUBLIC_DEBUG === '1' ? performance.now() : 0; ... - const endTime = performance.now(); - console.log(`Statistics fetched in ${endTime - startTime}ms:`, allStats); - console.log('Platform stats:', allStats.platformStats); - console.log('Asset metrics count:', allStats.assetMetrics.length); + if (process.env.NEXT_PUBLIC_DEBUG === '1') { + const endTime = performance.now(); + console.log(`Statistics fetched in ${endTime - startTime}ms:`, allStats); + console.log('Platform stats:', allStats.platformStats); + console.log('Asset metrics count:', allStats.assetMetrics.length); + }src/components/RiskNotificationModal.tsx (1)
40-40: Make non-dismissable intent explicitUsing onClose={() => {}} blocks close handlers implicitly. Prefer isDismissable={false} for clarity and to prevent backdrop/esc key closing.
Apply:
- <Modal isOpen={isOpen} onClose={() => {}} hideCloseButton size="3xl" scrollBehavior="inside"> + <Modal + isOpen={isOpen} + onClose={() => {}} + isDismissable={false} + hideCloseButton + size="3xl" + scrollBehavior="inside" + >app/markets/components/Pagination.tsx (2)
2-2: Minor: rename alias for clarityThe alias still says NextUI. Optional rename to avoid confusion during the HeroUI migration.
Apply:
-import { Pagination as NextUIPagination } from '@heroui/react'; +import { Pagination as HeroUIPagination } from '@heroui/react';And update usage:
- <NextUIPagination + <HeroUIPagination
31-33: Controlled Pagination: drop initialPageWith a controlled page prop, initialPage is redundant and can confuse future readers.
Apply:
- initialPage={1}app/market/[chainId]/[marketid]/RateChart.tsx (2)
4-5: UI lib migration looks fine; consider merging importsThe switch to @heroui/react for Card/Progress aligns with the PR goal. You can merge these two imports into one for tidiness.
-import { Card, CardHeader, CardBody } from '@heroui/react'; -import { Progress } from '@heroui/react'; +import { Card, CardHeader, CardBody, Progress } from '@heroui/react';
57-57: Remove stray console.logLeftover debug log can be noisy in production.
- console.log('market', market); + // console.log('market', market);app/rewards/components/RewardContent.tsx (1)
80-81: Fix small typos in commentsKeeps the codebase clean.
- // go through all posible keys of rewad object: for_supply, for_borrow, for_collateral} + // go through all possible keys of reward object: for_supply, for_borrow, for_collateralsrc/components/TokenIcon.tsx (1)
6-13: symbol prop is unused; consider using it for fallback alt/contentEither use symbol when token lookup fails (for better a11y) or remove the prop if it’s not needed.
Example usage in fallback:
// Fallback to placeholder return ( <div aria-label={symbol ?? 'token'} className="rounded-full bg-gray-300 dark:bg-gray-700" style={{ width, height }} /> );app/positions/components/agent/SetupAgent.tsx (1)
203-221: Simplify effect; the mounted flag is unnecessaryThe effect body is synchronous; the mounted guard doesn’t add value.
- useEffect(() => { - let mounted = true; + useEffect(() => { if (!hasPreselected && groupedMarkets.length > 0 && targetNetwork) { groupedMarkets.forEach((group) => { group.activeMarkets.forEach((market) => { if (!isInPending(market) && !isInPendingRemove(market)) { addToPendingCaps(market, maxUint256); } }); }); - if (mounted) { - setHasPreselected(true); - } + setHasPreselected(true); } - - return () => { - mounted = false; - }; }, [ hasPreselected, groupedMarkets, isInPending, isInPendingRemove, addToPendingCaps, targetNetwork, ]);app/market/[chainId]/[marketid]/components/PositionStats.tsx (1)
207-217: Make Switch controlled to avoid state driftUsing defaultSelected makes the Switch uncontrolled and can desync from viewMode over time. HeroUI supports isSelected and onValueChange returning the boolean.
Apply:
- <Switch - defaultSelected={viewMode === 'user'} + <Switch + isSelected={viewMode === 'user'} size="sm" color="primary" classNames={{ wrapper: 'mx-0', thumbIcon: 'p-0 mr-0', }} - onChange={toggleView} + onValueChange={(selected) => setViewMode(selected ? 'user' : 'global')} thumbIcon={ThumbIcon} />app/history/components/HistoryTable.tsx (1)
353-358: Use Next.js Link for internal navigation (optional)For client-side routing and prefetch, prefer next/link for internal routes.
- <Link - href={`/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`} - className="font-monospace text-xs hover:underline" - > + <NextLink + href={`/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`} + className="font-monospace text-xs hover:underline" + > {market.uniqueKey.slice(2, 8)} - </Link> + </NextLink>Also add this import at the top of the file:
import NextLink from 'next/link';tailwind.config.ts (1)
78-83: Stale selector: consider covering HeroUI’s button rootThe selector still targets .nextui-button. If HeroUI changed the root class, styles may not apply.
Apply this diff to safely cover both:
- 'button, .nextui-button': { + 'button, .nextui-button, .ui-button': { '@apply rounded': {}, // This makes all buttons rounded by default },If HeroUI doesn’t use .ui-button, keep as-is. Verify in the browser DevTools which class is present on Button root.
src/components/layout/header/AccountDropdown.tsx (1)
36-44: Improve accessibility of the triggerA div as trigger lacks semantics. Add role/button affordances.
- <div + <div + role="button" + aria-label="Open account menu" className={clsx( 'flex h-8 w-8 cursor-pointer items-center justify-center', 'transition-all duration-150 ease-in-out hover:-translate-y-[2px]', )} >src/components/common/DatePicker.tsx (1)
4-11: Type the classNames and allow overrides; spread remaining propsThis guards against key typos and reduces boilerplate when DatePicker surfaces new props.
-function StyledDatePicker(props: DatePickerProps) { - const classNames = { +function StyledDatePicker(props: DatePickerProps) { + const { classNames: classNamesProp, ...rest } = props; + const classNames: NonNullable<DatePickerProps['classNames']> = { base: 'h-[88px]', // Reserve space for error message inputWrapper: ['bg-surface', 'rounded-sm', 'w-64', 'h-14'], label: 'text-xs text-gray-500', input: 'text-sm', errorMessage: 'text-xs text-red-500 absolute bottom-0 left-0', selectorButton: ['hover:text-primary'], - }; + , ...(classNamesProp ?? {}) }; return ( <DatePicker - label={props.label} - variant={props.variant} - value={props.value} + {...rest} classNames={classNames} - minValue={props.minValue} - maxValue={props.maxValue} - onChange={props.onChange} - isInvalid={props.isInvalid} - errorMessage={props.errorMessage} - granularity={props.granularity} /> ); }Also applies to: 18-18
src/components/common/Button.tsx (1)
44-46: isDisabled should be boolean, not stringAs written, the compound variant may never match.
- { - isDisabled: 'true', - class: 'opacity-50 cursor-not-allowed pointer-events-none', - }, + { + isDisabled: true, + class: 'opacity-50 cursor-not-allowed pointer-events-none', + },package.json (1)
22-31: Audit @heroui/ imports*A scan of all .ts/.tsx files shows only
@heroui/reactis ever imported. You can safely remove these unused sub-packages from package.json to slim your bundle:• @heroui/accordion
• @heroui/button
• @heroui/checkbox
• @heroui/input
• @heroui/system
• @heroui/table
• @heroui/theme
• @heroui/tooltipapp/markets/components/markets.tsx (1)
174-192: Preserve unrelated query params and avoid pushing a bare “?”.Rebuilding from an empty URLSearchParams drops any unrelated query keys (e.g., utm) and pushing “?” when empty is noisy.
- const updateUrlParams = useCallback( - (collaterals: string[], loanAssets: string[], network: SupportedNetworks | null) => { - const params = new URLSearchParams(); - - if (collaterals.length > 0) { - params.set('collaterals', collaterals.join(',')); - } - if (loanAssets.length > 0) { - params.set('loanAssets', loanAssets.join(',')); - } - if (network) { - params.set('network', network.toString()); - } - - const newParams = params.toString(); - router.push(`?${newParams}`, { scroll: false }); - }, - [router], - ); + const updateUrlParams = useCallback( + (collaterals: string[], loanAssets: string[], network: SupportedNetworks | null) => { + const params = new URLSearchParams( + typeof window !== 'undefined' ? window.location.search : '', + ); + + if (collaterals.length > 0) params.set('collaterals', collaterals.join(',')); + else params.delete('collaterals'); + + if (loanAssets.length > 0) params.set('loanAssets', loanAssets.join(',')); + else params.delete('loanAssets'); + + if (network) params.set('network', network.toString()); + else params.delete('network'); + + const qs = params.toString(); + const path = + typeof window !== 'undefined' ? window.location.pathname : ''; + router.push(qs ? `${path}?${qs}` : path, { scroll: false }); + }, + [router], + );app/markets/page.tsx (1)
34-41: Trim/dedupe parsed lists.Small polish to avoid empty strings/duplicates leaking into client state.
- const collaterals = params.collaterals - ? params.collaterals.split(',').filter(Boolean) - : []; + const collaterals = params.collaterals + ? Array.from(new Set(params.collaterals.split(',').map(s => s.trim()).filter(Boolean))) + : []; - const loanAssets = params.loanAssets - ? params.loanAssets.split(',').filter(Boolean) - : []; + const loanAssets = params.loanAssets + ? Array.from(new Set(params.loanAssets.split(',').map(s => s.trim()).filter(Boolean))) + : [];app/positions/report/components/ReportTable.tsx (5)
4-5: Combine HeroUI imports; confirm Table API parityMerge these two imports into one and verify the Table classNames API matches HeroUI’s (e.g., "th", "wrapper" keys).
-import { DateValue } from '@heroui/react'; -import { Table, TableHeader, TableBody, TableColumn, TableRow, TableCell } from '@heroui/react'; +import { + DateValue, + Table, + TableHeader, + TableBody, + TableColumn, + TableRow, + TableCell, +} from '@heroui/react';
303-313: Add rel="noopener noreferrer" for external link securityTargeting _blank without rel is a security footgun. Add rel to prevent tabnabbing.
- <Link + <Link href={getExplorerTxURL( tx.hash, marketReport.market.morphoBlue.chain.id, )} target="_blank" + rel="noopener noreferrer" className="flex items-center gap-1 text-gray-500 hover:text-gray-700" >
24-33: Stop passing unused props to MarketSummaryBlock
amountandavgCapitalaren’t used insideMarketSummaryBlock. Drop them from the prop type and callsite.type MarketInfoBlockProps = { market: Market; interestEarned: bigint; - amount: bigint; decimals: number; symbol: string; apy: number; - avgCapital: bigint; hasActivePosition: boolean; };<MarketSummaryBlock market={marketReport.market} interestEarned={marketReport.interestEarned} - amount={marketReport.endBalance} decimals={asset.decimals} symbol={asset.symbol} apy={marketReport.apy} - avgCapital={marketReport.avgCapital} hasActivePosition={hasActivePosition} />Also applies to: 225-235
196-205: Avoid Number() coercion on BigInt during sortCoercing BigInt deltas to Number can overflow. Compare BigInts directly.
- // Then sort by interest earned - return Number(b.interestEarned - a.interestEarned); + // Then sort by interest earned + if (b.interestEarned === a.interestEarned) return 0; + return b.interestEarned > a.interestEarned ? 1 : -1;
59-61: Comment vs. code mismatch: actually trim trailing zerosYou say “remove trailing zeros” but return toFixed(2) unchanged. Implement trimming.
- // Otherwise, round to 2 decimal places and remove trailing zeros - return days.toFixed(2); + // Otherwise, round to 2 decimal places and remove trailing zeros + return parseFloat(days.toFixed(2)).toString();app/positions/report/components/ReportContent.tsx (2)
212-216: Remove any-casts around DatePicker; keep type-safetyAvoid disabling ESLint and casting to any. Adapt the handler inline to the expected DateValue.
- // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - value={startDate as any} - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - onChange={handleStartDateChange as any} + value={startDate as DateValue} + onChange={(d) => handleStartDateChange(d as ZonedDateTime)}- // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - value={endDate as any} - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - onChange={handleEndDateChange as any} + value={endDate as DateValue} + onChange={(d) => handleEndDateChange(d as ZonedDateTime)}If you’d like, I can help adjust the local DatePicker component to be generic over T extends DateValue so callers don’t need casts.
Also applies to: 225-229
167-181: Guard against positions being undefined in useMemo
positionscan be undefined (you treat it as such elsewhere). Prevent a runtime error.- positions.forEach((position) => { + const list = positions ?? []; + list.forEach((position) => { const key = `${position.market.loanAsset.address}-${position.market.morphoBlue.chain.id}`; if (!assetMap.has(key)) { assetMap.set(key, { symbol: position.market.loanAsset.symbol, address: position.market.loanAsset.address as Address, chainId: position.market.morphoBlue.chain.id, decimals: position.market.loanAsset.decimals, }); } });If
useUserPositionsalready guaranteesdatais[], feel free to skip—otherwise this avoids crashes on initial render.app/positions/components/onboarding/SetupPositions.tsx (3)
96-107: Use functional setState for Set to avoid stale closuresMinor, but this avoids capturing stale
lockedAmountsand is cheaper on re-renders.- const toggleLockAmount = useCallback( - (marketKey: string) => { - const newLockedAmounts = new Set(lockedAmounts); - if (lockedAmounts.has(marketKey)) { - newLockedAmounts.delete(marketKey); - } else { - newLockedAmounts.add(marketKey); - } - setLockedAmounts(newLockedAmounts); - }, - [lockedAmounts], - ); + const toggleLockAmount = useCallback((marketKey: string) => { + setLockedAmounts((prev) => { + const next = new Set(prev); + if (next.has(marketKey)) next.delete(marketKey); + else next.add(marketKey); + return next; + }); + }, []);
62-74: Avoid Number-based math for token amounts (precision loss)Using
Number(totalAmount)can lose precision for large values/decimals. Do math in units with BigInt and format back.- if (totalAmount && Object.keys(percentages).length > 0) { - const newAmounts = Object.entries(percentages).reduce( - (acc, [key, percentage]) => { - acc[key] = ((percentage / 100) * Number(totalAmount)).toFixed(tokenDecimals); - return acc; - }, - {} as Record<string, string>, - ); - setAmounts(newAmounts); - } + if (totalAmount && Object.keys(percentages).length > 0) { + try { + const totalUnits = parseUnits(totalAmount, tokenDecimals); + const newAmounts = Object.entries(percentages).reduce((acc, [key, percentage]) => { + const units = (totalUnits * BigInt(Math.round(percentage * 10000))) / BigInt(100 * 10000); + acc[key] = formatUnits(units, tokenDecimals); + return acc; + }, {} as Record<string, string>); + setAmounts(newAmounts); + } catch { + // ignore until totalAmount is valid + } + }
333-335: Defensive guard for Slider’s onChange value typeHeroUI’s onChange can be number | number[] (range). You’re not using range, but a tiny guard prevents accidental NaN if the prop changes.
- onChange={(value) => - handlePercentageChange(market.uniqueKey, Number(value)) - } + onChange={(value) => { + const v = Array.isArray(value) ? value[0] : value; + handlePercentageChange(market.uniqueKey, Number(v)); + }}src/components/Borrow/AddCollateralAndBorrow.tsx (3)
341-343: Use a wrapper to avoid unhandled rejections (consistency with other buttons)Elsewhere you wrap async calls with
() => void .... Do the same here for consistency and to avoid unhandled rejections ifswitchToNetworkreturns a Promise.- <Button onPress={switchToNetwork} className="min-w-32" variant="solid"> + <Button onPress={() => void switchToNetwork()} className="min-w-32" variant="solid">
225-233: Compare BigInt with BigInt literals (0n)Minor type-safety/consistency tweak: use
0nwhen comparing BigInt values.- borrowAmount > 0 || collateralAmount > 0 ? newLTV : currentLTV, + borrowAmount > 0n || collateralAmount > 0n ? newLTV : currentLTV,And similarly a few lines below:
- (Number(borrowAmount > 0 || collateralAmount > 0 ? newLTV : currentLTV) / + (Number(borrowAmount > 0n || collateralAmount > 0n ? newLTV : currentLTV) /
373-377: Strict equality for BigIntUse
===with BigInt to avoid implicit coercion.- {collateralAmount > 0n && borrowAmount == 0n ? 'Add Collateral' : 'Borrow'} + {collateralAmount > 0n && borrowAmount === 0n ? 'Add Collateral' : 'Borrow'}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (56)
.claude/settings.local.json(1 hunks).yarnrc.yml(0 hunks)app/admin/stats/components/AssetMetricsTable.tsx(1 hunks)app/admin/stats/components/StatsOverviewCards.tsx(1 hunks)app/admin/stats/page.tsx(1 hunks)app/history/components/HistoryTable.tsx(1 hunks)app/info/components/info.tsx(1 hunks)app/market/[chainId]/[marketid]/RateChart.tsx(1 hunks)app/market/[chainId]/[marketid]/VolumeChart.tsx(1 hunks)app/market/[chainId]/[marketid]/components/BorrowsTable.tsx(1 hunks)app/market/[chainId]/[marketid]/components/LiquidationsTable.tsx(1 hunks)app/market/[chainId]/[marketid]/components/PositionStats.tsx(1 hunks)app/market/[chainId]/[marketid]/components/SuppliesTable.tsx(1 hunks)app/market/[chainId]/[marketid]/content.tsx(2 hunks)app/markets/components/AdvancedSearchBar.tsx(1 hunks)app/markets/components/MarketRowDetail.tsx(1 hunks)app/markets/components/MarketSettingsModal.tsx(3 hunks)app/markets/components/MarketTableBody.tsx(2 hunks)app/markets/components/NetworkFilter.tsx(1 hunks)app/markets/components/Pagination.tsx(1 hunks)app/markets/components/RiskIndicator.tsx(1 hunks)app/markets/components/markets.tsx(8 hunks)app/markets/page.tsx(2 hunks)app/positions/components/FromAndToMarkets.tsx(2 hunks)app/positions/components/PositionsSummaryTable.tsx(3 hunks)app/positions/components/RebalanceCart.tsx(1 hunks)app/positions/components/RebalanceModal.tsx(2 hunks)app/positions/components/SuppliedMarketsDetail.tsx(3 hunks)app/positions/components/agent/Main.tsx(1 hunks)app/positions/components/agent/SetupAgent.tsx(1 hunks)app/positions/components/agent/SetupAgentModal.tsx(1 hunks)app/positions/components/onboarding/AssetSelection.tsx(1 hunks)app/positions/components/onboarding/Modal.tsx(2 hunks)app/positions/components/onboarding/RiskSelection.tsx(3 hunks)app/positions/components/onboarding/SetupPositions.tsx(1 hunks)app/positions/report/components/ReportContent.tsx(3 hunks)app/positions/report/components/ReportTable.tsx(1 hunks)app/rewards/components/InfoCard.tsx(1 hunks)app/rewards/components/RewardContent.tsx(1 hunks)app/rewards/components/RewardTable.tsx(2 hunks)app/settings/page.tsx(1 hunks)package.json(6 hunks)src/components/Borrow/AddCollateralAndBorrow.tsx(4 hunks)src/components/FeedInfo/OracleFeedInfo.tsx(1 hunks)src/components/OracleVendorBadge.tsx(1 hunks)src/components/RiskNotificationModal.tsx(1 hunks)src/components/SupplyModalContent.tsx(4 hunks)src/components/TokenIcon.tsx(1 hunks)src/components/common/Button.tsx(1 hunks)src/components/common/DatePicker.tsx(1 hunks)src/components/common/MarketInfoBlock.tsx(1 hunks)src/components/layout/header/AccountDropdown.tsx(2 hunks)src/components/layout/header/Navbar.tsx(1 hunks)src/components/providers/ThemeProvider.tsx(2 hunks)src/store/createWagmiConfig.ts(0 hunks)tailwind.config.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/store/createWagmiConfig.ts
- .yarnrc.yml
✅ Files skipped from review due to trivial changes (2)
- app/admin/stats/components/AssetMetricsTable.tsx
- app/rewards/components/InfoCard.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/components/SupplyModalContent.tsx
- app/positions/components/SuppliedMarketsDetail.tsx
- app/positions/components/onboarding/RiskSelection.tsx
- app/markets/components/MarketSettingsModal.tsx
- app/rewards/components/RewardTable.tsx
- app/positions/components/onboarding/Modal.tsx
- app/market/[chainId]/[marketid]/content.tsx
- app/positions/components/FromAndToMarkets.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-16T02:01:51.219Z
Learnt from: antoncoding
PR: antoncoding/monarch#97
File: app/positions/components/onboarding/OnboardingContext.tsx:36-43
Timestamp: 2024-12-16T02:01:51.219Z
Learning: In `app/positions/components/onboarding/OnboardingContext.tsx`, the `defaultStep` variable is no longer needed and can be removed.
Applied to files:
app/positions/components/onboarding/SetupPositions.tsx
📚 Learning: 2024-12-17T10:51:07.277Z
Learnt from: antoncoding
PR: antoncoding/monarch#99
File: src/components/AgentSetupProcessModal.tsx:38-51
Timestamp: 2024-12-17T10:51:07.277Z
Learning: In `src/components/AgentSetupProcessModal.tsx`, the modal is intentionally designed without a close button.
Applied to files:
app/positions/components/agent/SetupAgentModal.tsx
📚 Learning: 2024-10-12T09:23:16.495Z
Learnt from: antoncoding
PR: antoncoding/monarch#63
File: app/markets/components/MarketRowDetail.tsx:49-52
Timestamp: 2024-10-12T09:23:16.495Z
Learning: When rendering oracle feeds in `ExpandedMarketDetail` (`app/markets/components/MarketRowDetail.tsx`), prefer explicit rendering over iterating keys when dealing with a small number of feeds.
Applied to files:
app/markets/components/markets.tsx
🧬 Code Graph Analysis (3)
src/components/Borrow/AddCollateralAndBorrow.tsx (1)
src/components/common/Button.tsx (1)
Button(4-53)
app/markets/page.tsx (3)
app/positions/[account]/page.tsx (1)
MarketPage(12-14)app/info/page.tsx (1)
MarketPage(12-14)src/utils/networks.ts (1)
SupportedNetworks(52-52)
app/markets/components/markets.tsx (3)
src/utils/networks.ts (1)
SupportedNetworks(52-52)src/utils/tokens.ts (2)
ERC20Token(17-29)UnknownERC20Token(31-37)src/hooks/useLocalStorage.ts (1)
useLocalStorage(4-78)
🔇 Additional comments (51)
app/positions/components/onboarding/AssetSelection.tsx (2)
142-156: Confirm Tooltip prop compatibility after the swapHeroUI’s Tooltip generally keeps NextUI’s API. If styling doesn’t apply as expected, consider using
classNames={{ content: '...' }}orcontentPropsfor the overlay.
2-2: No leftover NextUI imports detectedHeroUI Tooltip import is correct, and no
@nextui-org/reactreferences remain.app/admin/stats/components/StatsOverviewCards.tsx (1)
2-2: No stray NextUI imports found—ready to merge.All Card/CardBody imports have been successfully switched to @heroui/react.
src/components/common/MarketInfoBlock.tsx (1)
2-2: Tooltip import migrated to HeroUI — API matches
contentprop and className usage align with @heroui/react Tooltip.app/info/components/info.tsx (1)
6-6: Button import moved to HeroUI and onPress is already in useProps (
isIconOnly,variant="flat",color="primary",onPress) match HeroUI’s API.app/markets/components/AdvancedSearchBar.tsx (1)
3-3: Input import migrated to HeroUI — props line up
onValueChange,endContent, andclassNameskeys (inputWrapper,input) are supported. The forwarded ref should still resolve to HTMLInputElement.If TypeScript complains about the ref type after the library swap, adjust to:
const inputRef = useRef<HTMLInputElement>(null);(then confirm no TS errors around the ref assignment).
app/markets/components/RiskIndicator.tsx (1)
1-1: Tooltip import swap to @heroui/react is fine
contentprop and styling remain compatible.app/market/[chainId]/[marketid]/VolumeChart.tsx (1)
4-4: Card import migrated to @heroui/react — looks goodNo behavior change; Card, CardHeader, CardBody usage is compatible.
src/components/OracleVendorBadge.tsx (1)
2-2: Tooltip import migrated to @heroui/react — looks goodUsage (content prop, className) remains compatible.
app/markets/components/NetworkFilter.tsx (1)
2-2: Select/SelectItem import migrated to @heroui/react — looks goodMatches the broader UI migration.
app/positions/components/agent/SetupAgentModal.tsx (2)
2-2: Modal import migrated to @heroui/react — looks goodNo structural changes; props and classNames remain the same.
165-165: Confirm close button prop compatibility with HeroUIDesign calls for no close button (learned from prior PR). Verify that
closeButton={false}is still respected in@heroui/react. If not, switch to the supported prop (oftenhideCloseButton) on the correct element (Modal or ModalContent), e.g.:<Modal ...> <ModalContent hideCloseButton className="p-4"> ... </ModalContent> </Modal>Also confirm backdrop/esc behavior matches expectations (no accidental dismissal if that’s undesired).
app/settings/page.tsx (1)
3-3: Switch import migrated to @heroui/react — looks good
defaultSelectedandonValueChangealign with the expected API.src/components/FeedInfo/OracleFeedInfo.tsx (1)
1-1: HeroUI Tooltip import looks fineImport switch aligns with the migration. No API differences surfaced here.
app/markets/components/MarketRowDetail.tsx (1)
1-1: HeroUI Tooltip import looks fineMatches the UI library migration.
app/admin/stats/page.tsx (1)
4-5: HeroUI imports: goodButton and Dropdown moved to @heroui/react; usage matches expected props.
src/components/RiskNotificationModal.tsx (1)
12-12: HeroUI import switch looks goodModal, Button, Checkbox props match the new package.
app/markets/components/Pagination.tsx (1)
2-2: HeroUI Pagination import: OKImport alias remains; component usage unchanged.
app/market/[chainId]/[marketid]/RateChart.tsx (1)
244-256: Double-check Progress API parity after library switchProps like showValueLabel and classNames.value/base/label should match HeroUI’s Progress API. Worth a quick sanity check since subtle prop name changes can slip in during library migrations.
src/components/layout/header/Navbar.tsx (1)
4-4: Dropdown import migration LGTM; verify event handling expectationsThe import swap to @heroui/react is consistent. Since other components migrated onClick→onPress/onAction, confirm DropdownItem still supports onClick here or consider centralizing with DropdownMenu onAction.
app/rewards/components/RewardContent.tsx (2)
4-4: Switch/Tooltip import migration LGTM; confirm prop compatibilityisSelected/onValueChange for Switch and content/placement for Tooltip should be API-compatible in HeroUI. Please confirm at runtime.
270-276: Nice: controlled Switch wiring is correctUsing isSelected + onValueChange keeps it controlled and predictable.
src/components/TokenIcon.tsx (1)
2-2: Tooltip import migration LGTMNo behavioral changes apparent.
app/positions/components/agent/SetupAgent.tsx (1)
9-9: UI lib import migration LGTMImporting Checkbox/Dropdown suite from @heroui/react matches the repo-wide change.
app/market/[chainId]/[marketid]/components/PositionStats.tsx (1)
2-3: UI import migration to @heroui/react looks goodCard and Switch swap reads clean. No other API mismatches spotted at a glance.
app/positions/components/agent/Main.tsx (1)
1-1: Tooltip migration to @heroui/react is fineProps usage (content, className) matches HeroUI’s Tooltip API.
app/market/[chainId]/[marketid]/components/BorrowsTable.tsx (1)
2-2: Table/Link/Pagination migration to @heroui/react looks correctTable props (classNames, bottomContent) and Pagination (onChange, page/total) align with HeroUI.
src/components/providers/ThemeProvider.tsx (1)
3-16: HeroUIProvider substitution LGTMProvider move to HeroUI is correct and placement within NextThemesProvider is fine.
app/history/components/HistoryTable.tsx (1)
3-3: Consolidated imports to @heroui/react look goodChip, Link, Pagination, and Table family from HeroUI are consistent with usage below.
app/market/[chainId]/[marketid]/components/LiquidationsTable.tsx (1)
2-2: HeroUI import swap looks goodConsolidated import aligns with the repo-wide migration. No usage issues spotted.
tailwind.config.ts (1)
6-6: Theme plugin/content migration looks correct
- heroui() plugin and content glob updated properly.
- Should integrate with the rest of the HeroUI changes without breaking token generation.
Also applies to: 12-13, 55-76
src/components/layout/header/AccountDropdown.tsx (1)
4-4: UI import migration LGTMDropdown components now sourced from @heroui/react; usage remains consistent.
src/components/common/DatePicker.tsx (1)
1-1: Import migration OKSwitch to @heroui/react matches the rest of the PR.
src/components/common/Button.tsx (1)
2-2: Import source updated — looks goodextendVariants is available from @heroui/react; no API changes here.
app/positions/components/RebalanceCart.tsx (1)
2-2: ✅ Dependency swap complete
- package.json lists “@heroui/react”: “^2.4.2” and no “@nextui-org/react”
- All component imports now reference “@heroui/react”
app/markets/components/MarketTableBody.tsx (2)
2-2: Tooltip import migrated to @heroui/reactLooks consistent with the broader UI migration.
170-179: Approve: Button handlers migrated to onPress
Repo-wide search found no<Button onClick=…>usages; the commonButtoncorrectly forwardsonPress. All set.app/positions/components/RebalanceModal.tsx (2)
2-2: Modal import switched to @heroui/reactProps align with Heroui Modal (isOpen, onClose, isDismissable, classNames). No behavioral changes.
302-310: Refresh Button: onPress migrationEvent change only; behavior intact. Spinner and disable logic are preserved.
app/positions/components/PositionsSummaryTable.tsx (1)
10-10: Heroui import consolidationDropdown/Tooltip/Switch and the aliased Button now come from @heroui/react. Reads clean.
app/market/[chainId]/[marketid]/components/SuppliesTable.tsx (1)
2-2: HeroUI import looks fine; double-check prop parity (Pagination/Table).Given the library swap, confirm Pagination’s onChange(page:number) and Table’s classNames/structure are still identical in @heroui/react to avoid subtle runtime/typing mismatches. If type-check passes here, you’re good.
package.json (1)
100-121: Node 22 compatibility check for test tooling.Jest 29.x historically lagged on brand-new Node majors. You may be fine, but verify locally/CI that jest, ts-jest, and jest-environment-jsdom run cleanly on Node 22.
app/markets/components/markets.tsx (2)
399-418: onPress migration looks correct.Consistent with the Button wrapper switch; matches other changes in the PR.
298-313: State-derived market link construction is solid.Keeps navigation consistent with current filters; opening in a new tab is intentional here.
app/markets/page.tsx (1)
12-24: Confirm Next 15’s searchParams-as-Promise usage; consider reverting to plain object for clarity.Awaiting a non-promise is harmless at runtime, but the Promise typing may confuse. If not required, simplify.
Option:
-type PageProps = { - searchParams: Promise<{ +type PageProps = { + searchParams?: { network?: string; collaterals?: string; loanAssets?: string; [key: string]: string | string[] | undefined; - }>; -} + }; +} -export default async function MarketPage({ searchParams }: PageProps) { - // Await the searchParams Promise in Next.js 15 - const params = await searchParams; +export default function MarketPage({ searchParams = {} }: PageProps) { + const params = searchParams;app/positions/report/components/ReportContent.tsx (1)
4-4: DateValue import migration looks goodImporting DateValue from @heroui/react aligns with the library switch.
app/positions/components/onboarding/SetupPositions.tsx (2)
2-2: Slider import migration looks goodHeroUI’s Slider API used here (value, onChange, minValue/maxValue, isDisabled, classNames) matches what's in this file.
210-218: SelectedToken non-null assertion is safeSetupPositions only renders once the onboarding flow advances to the ‘setup’ step, and the context’s
canGoNextguard won’t allow that untilselectedTokenis non-null (and markets are chosen). No change needed here.Likely an incorrect or invalid review comment.
src/components/Borrow/AddCollateralAndBorrow.tsx (3)
356-357: Good swap to onPress with void wrapper
onPress={() => void approveAndBorrow()}matches the new API and prevents unhandled Promise rejections.
372-373: Good swap to onPress with void wrapper
onPress={() => void signAndBorrow()}looks correct and consistent.
2-2: Confirmed @heroui/react install & no leftover NextUI imports
- package.json declares @heroui/react@^2.4.2
- No
@nextui-org/imports found- No legacy
<Button onClick=…>usage detected
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/store/createWagmiConfig.ts (2)
23-35: Move wallet selection into the factory to avoid module-scope window checks and trim SSR costPicking wallets at import time can cause SSR/CSR divergence and pulls all wallet code into the server bundle. Resolve wallets inside
createWagmiConfiginstead.Apply this diff to remove the module-scope constant:
-const wallets = (typeof window !== "undefined") ? [ - rabbyWallet, - metaMaskWallet, - rainbowWallet, - coinbaseWallet, - argentWallet, - injectedWallet, - trustWallet, - ledgerWallet, -] : [ - injectedWallet, -]; +// Wallets are resolved inside createWagmiConfig for SSR safety.Then, inside
createWagmiConfig, add this near the top (example; keep the rest unchanged):const wallets = typeof window !== 'undefined' ? [rabbyWallet, metaMaskWallet, rainbowWallet, coinbaseWallet, argentWallet, injectedWallet, trustWallet, ledgerWallet] : [injectedWallet];Also confirm that dropping the Safe connector is intentional and that no UI paths still expect it.
39-42: Nit: pluralize the group labelMinor copy tweak for clarity.
- groupName: 'Recommended Wallet', + groupName: 'Recommended Wallets',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
next.config.js(1 hunks)src/store/createWagmiConfig.ts(1 hunks)
🔇 Additional comments (1)
src/store/createWagmiConfig.ts (1)
39-42: RainbowKit v2 detected – no changes neededVerified you’re on RainbowKit 2.x (and wagmi 2.x), so passing wallet factories directly is correct. Just ensure
projectIdremains non-empty to keep WalletConnect happy.
There was a problem hiding this comment.
Actionable comments posted: 1> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limits.
🔭 Outside diff range comments (1)
package.json (1)
7-7: Update scripts to pnpm (packageManager is pnpm).Two scripts still invoke yarn.
Apply:
- "check": "yarn format && yarn lint && yarn stylelint", + "check": "pnpm format && pnpm lint && pnpm stylelint", ... - "test:coverage:open": "yarn test:coverage && open coverage/lcov-report/index.html" + "test:coverage:open": "pnpm test:coverage && open coverage/lcov-report/index.html"Optional: replace
openwith a cross‑platform tool if you care about Windows CI.Also applies to: 18-18
♻️ Duplicate comments (1)
package.json (1)
131-131: Declare Node engine and move pins to pnpm “overrides”.Engines help CI/dev match Node 22; with pnpm, use top‑level
overrides(notresolutions). This echoes a prior review.Apply:
"devDependencies": { ... "typescript": "~5.6.3" }, + "engines": { + "node": ">=22.12" + }, + "overrides": { + "tr46": "^4.0.0", + "punycode": "^2.3.1" + }, "packageManager": "pnpm@9.15.0"
🧹 Nitpick comments (1)
package.json (1)
6-6: Drop the pre-clean or use a cross‑platform cleaner.
rm -rf .nextis Unix-only and usually unnecessary;next buildhandles output. If you need a clean, prefer rimraf.Apply one of:
- "build": "rm -rf .next && next build", + "build": "next build",or
- "build": "rm -rf .next && next build", + "build": "rimraf .next && next build",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json(6 hunks)
🔇 Additional comments (2)
package.json (2)
22-30: Heroui migration deps look consistent.The @heroui/* set replaces NextUI cleanly and versions look aligned. No issues spotted here.
Also applies to: 36-36
20-77: Confirm removal of third‑party “net”.The problematic dependency is gone. Good cleanup.
| "typescript": "~5.6.3" | ||
| }, | ||
| "packageManager": "yarn@4.1.0" | ||
| "packageManager": "pnpm@9.15.0" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify lockfiles and manager switch.
Ensure repo uses pnpm lock and no stray yarn/npm locks remain.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Lockfiles present:"
fd -a 'pnpm-lock.yaml|yarn.lock|package-lock.json' -t f
echo
echo "Package manager declared in package.json:"
jq -r '.packageManager' package.json
echo
echo "Check scripts referencing yarn:"
rg -n -C1 -g '!**/node_modules/**' -P '\byarn\b' || true
echo
echo "Expectations:"
echo "- pnpm-lock.yaml present"
echo "- No yarn.lock or package-lock.json"
echo "- No scripts still invoking yarn (after fixes)"Length of output: 1323
Fix stray yarn invocations in package scripts
The repo correctly declares pnpm@9.15.0 and only pnpm-lock.yaml exists, but there are still two scripts calling yarn. Update them to use pnpm (or npm run) instead:
• package.json
– Line 7: "check": "yarn format && yarn lint && yarn stylelint"
– Line 18: "test:coverage:open": "yarn test:coverage && open coverage/lcov-report/index.html"
Suggested diff:
--- a/package.json
+++ b/package.json
@@ -6,4 +6,4 @@
"build": "rm -rf .next && next build",
- "check": "yarn format && yarn lint && yarn stylelint",
+ "check": "pnpm format && pnpm lint && pnpm stylelint",
"dev": "next dev",
@@ -17,4 +17,4 @@
"test:coverage": "jest . --coverage",
- "test:coverage:open": "yarn test:coverage && open coverage/lcov-report/index.html"
+ "test:coverage:open": "pnpm test:coverage && open coverage/lcov-report/index.html"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "packageManager": "pnpm@9.15.0" | |
| "scripts": { | |
| "build": "rm -rf .next && next build", | |
| "check": "pnpm format && pnpm lint && pnpm stylelint", | |
| "dev": "next dev", | |
| "test:coverage": "jest . --coverage", | |
| "test:coverage:open": "pnpm test:coverage && open coverage/lcov-report/index.html" | |
| } |
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores