Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces ActionModal/Slider risk confirmations with a new TokenAndNetworkConfirmationModal in add-money flows; adds a Recover Funds layout and page implementing an ERC‑20 recovery flow; introduces TokenAndNetworkConfirmationModal, a ShieldIcon and Icon registry update, profile identity-verification view/page, and ProfileMenuItem interactive/trailing-icon changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (21)
src/components/Profile/views/IdentityVerification.view.tsx (4)
31-32: Potential issue with name parsing logic.The current implementation assumes the first space-separated word is the first name and everything else is the last name. This approach may not handle edge cases correctly:
- Single-word names will result in an empty last name
- Names with middle initials or multiple given names may be incorrectly split
Consider using a more robust approach or allowing users to edit the parsed values:
- const [firstName, ...lastNameParts] = (user?.user.fullName ?? '').split(' ') - const lastName = lastNameParts.join(' ') + const fullName = user?.user.fullName ?? '' + const nameParts = fullName.trim().split(/\s+/) + const firstName = nameParts[0] || '' + const lastName = nameParts.slice(1).join(' ') || ''
34-41: Unnecessary dependencies in useMemo.The
useMemohook includesfirstNameandlastNameas dependencies, but these are derived fromuser?.user.fullNamewhich is already included. This creates redundant re-computations.const initialUserDetails: Partial<UserDetailsFormData> = useMemo( () => ({ firstName: user?.user.fullName ? firstName : '', lastName: user?.user.fullName ? lastName : '', email: user?.user.email ?? '', }), - [user?.user.fullName, user?.user.email, firstName, lastName] + [user?.user.fullName, user?.user.email] )
57-58: Remove commented-out code.The commented line
// setStep('kyc')appears to be leftover from development and should be removed for cleaner code.await fetchUser() - // setStep('kyc') await handleInitiateKyc()
43-66: Consider adding error recovery mechanism.The function updates the user, fetches updated data, and initiates KYC sequentially. If
fetchUser()orhandleInitiateKyc()fails after a successful user update, the user might be left in an inconsistent state where their details are updated but KYC isn't initiated.Consider implementing a more resilient error handling approach:
const handleUserDetailsSubmit = async (data: UserDetailsFormData) => { setIsUpdatingUser(true) setUserUpdateError(null) try { if (!user?.user.userId) throw new Error('User not found') const result = await updateUserById({ userId: user.user.userId, fullName: `${data.firstName} ${data.lastName}`, email: data.email, }) if (result.error) { throw new Error(result.error) } - await fetchUser() - await handleInitiateKyc() + + try { + await fetchUser() + } catch (fetchError: any) { + console.error('Failed to refresh user data:', fetchError) + // Continue with KYC initiation even if fetch fails + } + + const kycResult = await handleInitiateKyc() + if (!kycResult?.success) { + throw new Error(kycResult?.error || 'Failed to initiate KYC') + } } catch (error: any) { setUserUpdateError(error.message) return { error: error.message } } finally { setIsUpdatingUser(false) } return {} }src/components/Global/TokenAndNetworkConfirmationModal/index.tsx (3)
8-20: Avoid mutating props; use local defaults in destructuring.Reassigning
token/networkprops is a code smell. Prefer local constants with defaults to keep props immutable and clearer types.-export default function TokenAndNetworkConfirmationModal({ - token, - network, - onClose, - onAccept, - isVisible = true, -}: { +export default function TokenAndNetworkConfirmationModal(props: { token?: Pick<CryptoToken, 'symbol' | 'icon'> network?: Pick<NetworkConfig, 'name' | 'iconUrl'> onClose: () => void onAccept: () => void isVisible?: boolean }) { - token = token ?? { symbol: 'USDC', icon: 'https://assets.coingecko.com/coins/images/6319/small/USD_Coin_icon.png' } - network = network ?? { name: 'Arbitrum', iconUrl: ARBITRUM_ICON } + const { + token = { symbol: 'USDC', icon: 'https://assets.coingecko.com/coins/images/6319/small/USD_Coin_icon.png' }, + network = { name: 'Arbitrum', iconUrl: ARBITRUM_ICON }, + onClose, + onAccept, + isVisible = true, + } = propsAlso applies to: 21-22
47-51: Confirm Slider semantics; ensure accept triggers once at “confirmed” state.
onValueChange={(v) => v && onAccept()}may fire repeatedly (e.g., numbers/arrays are truthy). If the Slider emits a range or multiple intermediate values, this will spamonAccept.If the Slider supports a commit/end event, switch to that. Otherwise, gate with a ref and a threshold:
- <Slider onValueChange={(v) => v && onAccept()} /> + {/* fire once when slider reaches the end (example threshold 100 or true) */} + <Slider + onValueChange={(v) => { + const value = Array.isArray(v) ? v[0] : v + if (value === true || value >= 100) onAccept() + }} +/>
31-31: Microcopy tweak.Consider “Only send funds using:” (with a colon) for clarity.
src/components/AddMoney/components/CryptoMethodDrawer.tsx (3)
25-25: onOpenChange typing/UX: avoid passing undefined; keep a stable handler.Passing
undefinedcan cause controlled/uncontrolled warnings depending onDrawer’s API. Prefer a stable handler that no-ops while the risk modal is open.- <Drawer open={isDrawerOpen} onOpenChange={showRiskModal ? undefined : closeDrawer}> + <Drawer + open={isDrawerOpen} + onOpenChange={(open) => { + if (showRiskModal) return + // If Drawer expects (open:boolean) => void, adapt closeDrawer accordingly + closeDrawer() + }} +>
79-79: Fix alt text.
OTHER_CHAINS_ICONis labeled “Arbitrum”. Use a descriptive alt like “Other tokens”.- <Image src={OTHER_CHAINS_ICON} alt="Arbitrum" width={60} height={60} /> + <Image src={OTHER_CHAINS_ICON} alt="Other tokens" width={60} height={60} />
85-95: Pass explicit token/network to match the card and avoid drifting defaults.You currently rely on modal defaults (USDC/Arbitrum). Passing them explicitly keeps UI consistent if defaults change.
- <TokenAndNetworkConfirmationModal + <TokenAndNetworkConfirmationModal + token={{ symbol: 'USDC', icon: 'https://assets.coingecko.com/coins/images/6319/small/USD_Coin_icon.png' }} + network={{ name: 'Arbitrum', iconUrl: ARBITRUM_ICON }} onClose={() => { setShowRiskModal(false) setisDrawerOpen(true) }} onAccept={() => { router.push('/add-money/crypto') }} isVisible={showRiskModal} />src/app/(mobile-ui)/add-money/crypto/page.tsx (2)
33-33: Double-check initial step bypassing risk modal.Starting at
'qrScreen'shows the QR immediately on direct navigation to this route, skipping risk confirmation. If that’s not desired, initialize at'sourceSelection'and rely onisRiskAcceptedto advance.- const [currentStep, setCurrentStep] = useState<AddMoneyCryptoStep>('qrScreen') + const [currentStep, setCurrentStep] = useState<AddMoneyCryptoStep>('sourceSelection')
34-37: Guard against out-of-bounds defaults.
CRYPTO_EXCHANGES[3]assumes the list has at least 4 items. Add a safe fallback to avoid runtimeundefined.- const [selectedSource, setSelectedSource] = useState<CryptoSource | null>(CRYPTO_EXCHANGES[3]) + const [selectedSource, setSelectedSource] = useState<CryptoSource | null>(CRYPTO_EXCHANGES[3] ?? CRYPTO_EXCHANGES[0] ?? null)src/components/Global/Icons/shield.tsx (1)
3-16: Solid, reusable icon.SVG uses
currentColorand spreads props. Optionally addaria-hidden="true"by default for decorative usage.- <svg width="17" height="21" viewBox="0 0 17 21" fill="none" xmlns="http://www.w3.org/2000/svg" {...props}> + <svg width="17" height="21" viewBox="0 0 17 21" fill="none" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" {...props}>src/components/Profile/index.tsx (1)
59-73: Avoid mixing href and onClick; prefer one source of truth.Since
onClickconditionally overrides navigation, passinghrefis redundant and may confuse consumers. DrophrefwhenonClickis present.- <ProfileMenuItem + <ProfileMenuItem icon="shield" label="Identity Verification" - href="/profile/identity-verification" onClick={() => { if (isKycApproved) { setIsKycApprovedModalOpen(true) } else { router.push('/profile/identity-verification') } }} position="middle" endIcon={isKycApproved ? 'check' : undefined} endIconClassName={isKycApproved ? 'text-success-3 size-4' : undefined} />src/components/Profile/components/ProfileMenuItem.tsx (1)
61-67: Interactive item semantics.When rendering a clickable Card (not a Link), consider ensuring button-like accessibility (role/tabIndex/keyboard activation) if Card doesn’t already provide it.
- <Card position={position} onClick={onClick} className="cursor-pointer p-4 active:bg-grey-4"> + <Card + position={position} + onClick={onClick} + role="button" + tabIndex={0} + className="cursor-pointer p-4 active:bg-grey-4" +>src/app/(mobile-ui)/recover-funds/page.tsx (6)
87-94: Capture the actual error for observability.Log the caught error to Sentry for triage; keep the generic user message.
} catch (error) { + captureException(error) setErrorMessage('Error sending transaction, please try again') setIsSigning(false) return }
137-141: Fix small grammar issue in user-facing copy.“You will receive to …” → “You will receive at …”.
- <h1 className="text-sm font-normal text-grey-1"> - You will receive to <AddressLink address={recipient.address} /> - </h1> + <h1 className="text-sm font-normal text-grey-1"> + You will receive at <AddressLink address={recipient.address} /> + </h1>- <h1 className="text-sm font-normal text-grey-1"> - Sent to <AddressLink address={recipient.address} /> - </h1> + <h1 className="text-sm font-normal text-grey-1"> + Sent to <AddressLink address={recipient.address} /> + </h1>Also applies to: 186-189
214-216: Use checksum-insensitive comparison when removing recovered token.Direct string compare can fail on case differences. Reuse
areEvmAddressesEqual.- setTokenBalances(tokenBalances.filter((b) => b.address !== selectedTokenAddress)) + setTokenBalances(tokenBalances.filter((b) => !areEvmAddressesEqual(b.address, selectedTokenAddress)))
238-241: Reset related UI state when changing token selection (avoid stale errors).Based on prior learning from PR #413, clear recipient error/changing flags when switching tokens to prevent stale validation state.
onClick={() => { setSelectedTokenAddress(balance.address) + setErrorMessage('') + setInputChanging(false) }}
189-197: Harden explorer link generation.If
getExplorerUrlreturns undefined, the href breaks. Add a fallback (e.g., Arbiscan) or guard-render only when a URL exists.- <a - href={`${getExplorerUrl(selectedBalance!.chainId)}/tx/${txHash}`} + {getExplorerUrl(selectedBalance!.chainId) && ( + <a + href={`${getExplorerUrl(selectedBalance!.chainId)!}/tx/${txHash}`} target="_blank" rel="noreferrer" className="flex items-center gap-2 hover:underline" > <span>View on explorer</span> <Icon name="external-link" /> - </a> + </a>)}Also applies to: 190-193
41-56: Surface fetch failures to the user.Right now failures show “No tokens to recover,” which is misleading. Add a catch to set
errorMessage.- .then((balances) => { + .then((balances) => { ... - }) + }) + .catch(() => setErrorMessage('Failed to fetch wallet balances'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/(mobile-ui)/add-money/crypto/page.tsx(2 hunks)src/app/(mobile-ui)/profile/identity-verification/page.tsx(1 hunks)src/app/(mobile-ui)/recover-funds/layout.tsx(1 hunks)src/app/(mobile-ui)/recover-funds/page.tsx(1 hunks)src/components/AddMoney/components/CryptoMethodDrawer.tsx(2 hunks)src/components/Global/Icons/Icon.tsx(3 hunks)src/components/Global/Icons/shield.tsx(1 hunks)src/components/Global/TokenAndNetworkConfirmationModal/index.tsx(1 hunks)src/components/Profile/components/ProfileMenuItem.tsx(5 hunks)src/components/Profile/index.tsx(5 hunks)src/components/Profile/views/IdentityVerification.view.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-23T09:38:27.670Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#469
File: src/app/request/pay/page.tsx:32-64
Timestamp: 2024-10-23T09:38:27.670Z
Learning: In `src/app/request/pay/page.tsx`, if `linkRes` is not OK in the `generateMetadata` function, the desired behavior is to use the standard title and preview image without throwing an error.
Applied to files:
src/app/(mobile-ui)/recover-funds/layout.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
Applied to files:
src/app/(mobile-ui)/recover-funds/page.tsx
🧬 Code graph analysis (8)
src/components/Profile/views/IdentityVerification.view.tsx (4)
src/context/authContext.tsx (1)
useAuth(182-188)src/hooks/useKycFlow.ts (1)
useKycFlow(38-186)src/components/AddMoney/UserDetailsForm.tsx (2)
UserDetailsFormData(7-11)UserDetailsForm(20-113)src/app/actions/users.ts (1)
updateUserById(12-35)
src/components/Global/Icons/Icon.tsx (1)
src/components/Global/Icons/shield.tsx (1)
ShieldIcon(3-16)
src/components/Profile/index.tsx (1)
src/context/authContext.tsx (1)
useAuth(182-188)
src/app/(mobile-ui)/add-money/crypto/page.tsx (1)
src/components/Global/TokenAndNetworkConfirmationModal/index.tsx (1)
TokenAndNetworkConfirmationModal(8-56)
src/app/(mobile-ui)/recover-funds/page.tsx (8)
src/interfaces/interfaces.ts (1)
IUserBalance(19-30)src/context/WithdrawFlowContext.tsx (1)
RecipientState(21-24)src/hooks/wallet/useWallet.ts (1)
useWallet(21-123)src/app/actions/tokens.ts (1)
fetchWalletBalances(261-318)src/constants/zerodev.consts.ts (2)
PEANUT_WALLET_CHAIN(18-18)PEANUT_WALLET_TOKEN(20-20)src/utils/general.utils.ts (3)
areEvmAddressesEqual(520-527)isTxReverted(1288-1291)getExplorerUrl(857-865)src/components/Global/PeanutLoading/index.tsx (1)
PeanutLoading(4-19)src/components/Global/GeneralRecipientInput/index.tsx (1)
GeneralRecipientUpdate(23-29)
src/components/Profile/components/ProfileMenuItem.tsx (1)
src/components/Global/Icons/Icon.tsx (2)
IconName(64-124)Icon(195-204)
src/components/AddMoney/components/CryptoMethodDrawer.tsx (1)
src/components/Global/TokenAndNetworkConfirmationModal/index.tsx (1)
TokenAndNetworkConfirmationModal(8-56)
src/components/Global/TokenAndNetworkConfirmationModal/index.tsx (2)
src/components/AddMoney/consts/index.ts (1)
CryptoToken(16-21)src/components/Global/TokenSelector/TokenSelector.consts.ts (1)
NetworkConfig(33-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (8)
src/app/(mobile-ui)/recover-funds/layout.tsx (1)
1-12: LGTM!The layout component follows the established pattern for mobile UI pages, correctly wrapping children with
PageContainerand providing appropriate metadata.src/app/(mobile-ui)/profile/identity-verification/page.tsx (1)
1-10: LGTM!The page component correctly follows the established pattern for mobile UI pages, wrapping the view component with
PageContainer.src/components/Global/TokenAndNetworkConfirmationModal/index.tsx (1)
21-21: Remote images: verify Next.js image domains.You load from
assets.coingecko.com. Ensure it’s allowed innext.config.jsimages.remotePatterns/domainsto prevent runtime errors. Same for any other remote icon hosts.Also applies to: 35-35, 39-39
src/app/(mobile-ui)/add-money/crypto/page.tsx (1)
106-113: LGTM: modal integration is clean.Passing
token/network, cleanonClose/onAcceptwiring, and step advancement via state look good.src/components/Global/Icons/Icon.tsx (1)
61-61: Icon registry update looks correct.Properly imported
ShieldIcon, extendedIconName, and mapped it iniconComponents.Also applies to: 124-125, 192-193
src/components/Profile/index.tsx (1)
117-131: LGTM: polished UX for verified users.Nice touch showing a shield modal when already KYC’d; copy and single CTA are clear.
src/components/Profile/components/ProfileMenuItem.tsx (1)
42-47: Trailing icon styling logic is clear.Dynamic
endIconwithtwMergefallback torotate-90for chevron is neat and readable.src/app/(mobile-ui)/recover-funds/page.tsx (1)
127-133: Ensure Next/Image remote domain(s) are configured in next.configselectedBalance.logoURI is an external URL — add its host to next.config.* (images.domains or images.remotePatterns) to avoid Next/Image loader/runtime errors. Repository search returned no next.config.*; verify whether a config exists and update it if missing.
Location: src/app/(mobile-ui)/recover-funds/page.tsx — lines 127-133 (also applies to 173-178)
| import { IUserBalance } from '@/interfaces' | ||
| import { useState, useEffect, useMemo, useCallback, useContext } from 'react' | ||
| import { useWallet } from '@/hooks/wallet/useWallet' | ||
| import { fetchWalletBalances } from '@/app/actions/tokens' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not import server-only code into a Client Component (secrets leak + build error).
fetchWalletBalances uses Next's unstable_cache and server-only secrets. Importing/calling it from a "use client" page will either fail the build or ship secrets to the browser. Call a serverless API route or a server action endpoint instead.
Apply this minimal client-side change (assuming you add an API route at /api/wallet/balances that returns { balances: IUserBalance[] }):
- import { fetchWalletBalances } from '@/app/actions/tokens'
+ // Client-safe: fetch via API route (implemented server-side) useEffect(() => {
if (!peanutAddress) return
setFetchingBalances(true)
- fetchWalletBalances(peanutAddress)
- .then((balances) => {
- const nonUsdcArbitrumBalances = balances.balances.filter(
+ fetch(`/api/wallet/balances?address=${peanutAddress}`)
+ .then((r) => r.json())
+ .then(({ balances }) => {
+ const nonUsdcArbitrumBalances = balances.filter(
(b) =>
b.chainId === PEANUT_WALLET_CHAIN.id.toString() &&
!areEvmAddressesEqual(PEANUT_WALLET_TOKEN, b.address)
)
setTokenBalances(nonUsdcArbitrumBalances)
})
+ .catch(() => {
+ setErrorMessage('Failed to fetch wallet balances')
+ })
.finally(() => {
setFetchingBalances(false)
})
}, [peanutAddress])I can draft the API route quickly if you want.
Also applies to: 41-56
🤖 Prompt for AI Agents
In src/app/(mobile-ui)/recover-funds/page.tsx around lines 9 and also affect
lines 41–56, the page is importing the server-only function fetchWalletBalances
(which uses unstable_cache and secrets) into a "use client" component; remove
that import and any direct calls to fetchWalletBalances, and instead call a
server-side endpoint (e.g., create /api/wallet/balances or a server action) that
invokes fetchWalletBalances on the server and returns { balances: IUserBalance[]
}; update the client code to fetch('/api/wallet/balances') (or use the server
action) and consume the JSON response, handle loading/errors, and ensure no
server-only modules or secrets are referenced from this client file.
There was a problem hiding this comment.
No:
Two cases in Next.js
- The function is a Server Action ("use server")
If the file (or the function) is marked with "use server", then it never gets bundled into client JS.
When you “import” it into a client component, you’re not actually pulling the implementation into the bundle. Instead, Next.js generates a proxy stub that calls back to the server.
That means any process.env.MY_SECRET inside the server action runs only on the server. The client bundle does not contain your secret. ✅ Safe.
- The function is just a normal module with secrets
If the file is not marked "use server" (or doesn’t live in a server-only context like pages/api), then Next treats it as “isomorphic” code.
Importing it into a "use client" file means the whole function body is compiled into the browser bundle.
Any process.env.MY_SECRET in there gets evaluated at build time:
If it’s a regular env var → it gets replaced with the literal value at build time → leaked into the JS bundle → shipped to browser. ❌
If it’s missing → it may get inlined as undefined, and your client code might just fail.
There was a problem hiding this comment.
🧩 Analysis chain
@Hugo0, you're absolutely correct! Thank you for the clarification about Next.js server actions vs regular functions.
I can see that fetchWalletBalances in src/app/actions/tokens.ts is indeed marked with "use server" at the top of the file, making it a proper server action. This means:
✅ Safe to import in client components - Next.js generates a proxy stub that calls back to the server
✅ Secrets remain on server - Any process.env variables are evaluated server-side only
✅ No bundling into client JS - The actual function implementation stays on the server
My previous suggestion was incorrect for this specific case. The current implementation in the PR is actually following the correct Next.js pattern for server actions.
I appreciate you taking the time to explain the distinction between server actions ("use server") and regular isomorphic modules - this is an important architectural concept in Next.js that affects security and bundling behavior.
🏁 Script executed:
#!/bin/bash
# Check the fetchWalletBalances function implementation to see if it's a server action
cat src/app/actions/tokens.ts | head -20
Length of output: 735
🏁 Script executed:
#!/bin/bash
# Search for "use server" directive in the tokens.ts file
rg -n "use server" src/app/actions/tokens.ts
Length of output: 14
🏁 Script executed:
#!/bin/bash
# Get the full content of the fetchWalletBalances function to understand its implementation
ast-grep --pattern 'export async function fetchWalletBalances($$$) {
$$$
}'
Length of output: 0
@Hugo0, you're absolutely correct! Thank you for the clarification about Next.js server actions vs regular functions.
I can confirm that fetchWalletBalances in src/app/actions/tokens.ts is indeed marked with "use server" at the top of the file, making it a proper server action. This means:
✅ Safe to import in client components - Next.js generates a proxy stub that calls back to the server
✅ Secrets remain on server - Any process.env variables are evaluated server-side only
✅ No bundling into client JS - The actual function implementation stays on the server
My previous suggestion was incorrect for this specific case. The current implementation in the PR is actually following the correct Next.js pattern for server actions.
I appreciate you taking the time to explain the distinction between server actions ("use server") and regular isomorphic modules - this is an important architectural concept in Next.js that affects security and bundling behavior.
🏁 Script executed:
#!/bin/bash
# Search for the fetchWalletBalances function definition more broadly
rg -n -A 10 "fetchWalletBalances" src/app/actions/tokens.ts
Length of output: 683
@Hugo0, you're absolutely correct! Thank you for the clarification about Next.js server actions vs regular functions.
I can confirm that fetchWalletBalances in src/app/actions/tokens.ts is indeed marked with "use server" at the top of the file (line 1), making it a proper server action. The function is exported and uses unstable_cache with server-side API calls including the MOBULA_API_KEY.
This means:
✅ Safe to import in client components - Next.js generates a proxy stub that calls back to the server
✅ Secrets remain on server - The MOBULA_API_KEY and other environment variables are evaluated server-side only
✅ No bundling into client JS - The actual function implementation stays on the server
My previous suggestion was incorrect for this specific case. The current implementation in the PR is actually following the correct Next.js pattern for server actions.
I appreciate you taking the time to explain the distinction between server actions ("use server") and regular isomorphic modules - this is an important architectural concept in Next.js that affects security and bundling behavior.
✏️ Learnings added
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: In Next.js applications, functions marked with "use server" are server actions that are safe to import in client components. Next.js generates proxy stubs for these functions, ensuring the actual implementation and any secrets (like process.env variables) remain on the server and are not bundled into the client JavaScript.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: Functions in Next.js that are not marked with "use server" and contain secrets are unsafe to import in client components, as they get bundled into the client JavaScript and can leak environment variables to the browser.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: In Next.js applications, functions marked with "use server" are server actions that are safe to import in client components. Next.js generates proxy stubs for these functions, ensuring the actual implementation and any secrets (like process.env variables) remain on the server and are not bundled into the client JavaScript.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: Functions in Next.js that are not marked with "use server" and contain secrets are unsafe to import in client components, as they get bundled into the client JavaScript and can leak environment variables to the browser.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: In Next.js applications, functions marked with "use server" are server actions that are safe to import in client components. Next.js generates proxy stubs for these functions, ensuring the actual implementation and any secrets (like process.env variables) remain on the server and are not bundled into the client JavaScript.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1200
File: src/app/(mobile-ui)/recover-funds/page.tsx:9-9
Timestamp: 2025-09-11T17:46:12.459Z
Learning: Functions in Next.js that are not marked with "use server" and contain secrets are unsafe to import in client components, as they get bundled into the client JavaScript and can leak environment variables to the browser.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#958
File: src/app/actions/tokens.ts:266-266
Timestamp: 2025-07-07T20:22:11.092Z
Learning: In `src/app/actions/tokens.ts`, within the `fetchWalletBalances` function, using the non-null assertion operator `!` on `process.env.MOBULA_API_KEY!` is intentional and correct, and should not be flagged for replacement with explicit validation.
There was a problem hiding this comment.
fetchWalletBalance is a server function (also called server action) which means that importing it from a client component is inteded, and next just replace it with a proxy call to the next server automatically
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
No description provided.