Conversation
nikhilkumar1612
requested review from
IAmKio and
RanaBug
and removed request for
IAmKio
WalkthroughAdds a new "Pulse" frontend app (UI, components, hooks, utilities, types, styles, Tailwind config) with Etherspot Intent & Modular SDK integration; updates build config, dependencies, ESLint ignores, and fixes a README typo. No backend/API contract behavior changes beyond type additions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PulseApp
participant Search
participant Buy
participant ModularSDK
participant IntentSDK
participant API
User->>PulseApp: Open Pulse
PulseApp->>Search: render/search UI
User->>Search: type / select token
Search->>API: fetch tokens (debounced)
API-->>Search: return tokens
Search->>PulseApp: token selected
PulseApp->>Buy: render buy UI with token
User->>Buy: enter USD amount, choose paying token
Buy->>ModularSDK: check/install modules
ModularSDK-->>Buy: modules installed? status
Buy->>IntentSDK: expressIntent(intent)
IntentSDK-->>Buy: express intent response (bids)
User->>Buy: confirm -> shortlist bid
IntentSDK-->>Buy: bid/tx updates
Buy->>PulseApp: update UI/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
Deploying x with
|
| Latest commit: |
c67305b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9234dc7f.x-e62.pages.dev |
| Branch Preview URL: | https://ft-pulse.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🧹 Nitpick comments (26)
src/apps/pulse/hooks/useTokenSearch.ts (1)
17-27: Consider returning the error state.The
erroris destructured from the query but not returned from the hook, which may limit error handling capabilities in consuming components.return { searchText, setSearchText, searchData, isSearchLoading, isFetching, + error, }src/apps/pulse/constants/tokens.ts (1)
8-16: Empty logo entries risk broken<img>sourcesMost chains resolve to
'', which will render broken images in React (<img src="" />).
Either provide a placeholder/logo CDN URL or omit the key until a valid asset exists.-Polygon: "", -Base: "", -XDAI: "", -... +Polygon: "https://cryptologos.cc/logos/polygon-matic-logo.svg", +Base: "https://cryptologos.cc/logos/base-base-logo.svg", +...Alternatively, wrap consumers with a fallback that hides the icon when the URL is falsy.
src/apps/pulse/tailwind.pulse.config.js (1)
10-15: Breakpoint overlap may cause style collisions
desktopstarts atmin:1024px, whiletabletgoes up tomax:1024px; both hit exactly 1024 px.
Considermax:1023pxfortablet(orminsyntax for all) to avoid duplicate class evaluation.- tablet: { max: '1024px' }, + tablet: { max: '1023px' },src/apps/pulse/manifest.json (1)
1-5: Provide at least a default locale intranslationsAn empty object breaks look-ups in i18n helpers that expect keys.
If translations are not ready, seed with"en": {}or remove the property until needed.src/apps/pulse/index.tsx (1)
1-8: Minor: add explicitReact.FCtype and file headerNot required, yet typing the component plus a brief comment helps newcomers.
-import AppWrapper from "./components/AppWrapper"; -import './styles/tailwindPulse.css'; +/** + * Root entry for Pulse app. + */ +import React from 'react'; +import AppWrapper from './components/AppWrapper'; +import './styles/tailwindPulse.css'; -const App = () => { +const App: React.FC = () => {src/apps/pulse/components/Close.tsx (2)
1-1: Define a proper TypeScript interface for props.Consider defining a proper interface for better type safety and documentation, similar to the pattern used in
Esc.tsx.+interface Props { + onClose: () => void; +} + -export default function Close(props: {onClose: () => void}) { +export default function Close(props: Props) {
3-3: Use consistent styling approach and simplify onClick handler.The component mixes inline styles with Tailwind classes, which is inconsistent with the app's styling approach. Also, the arrow function wrapper in onClick is unnecessary.
- <button style={{height: 20, width: 20, marginRight: 10}} onClick={() => props.onClose()}> + <button className="h-5 w-5 mr-2.5" onClick={props.onClose}>src/apps/pulse/styles/tailwindPulse.css (2)
50-52: Use CSS custom properties or Tailwind color variables for consistency.Hardcoded hex values should be replaced with CSS custom properties or Tailwind color variables to maintain design system consistency and enable easier theming.
- --tw-ring-color: '#5e00ff'; - border-color: '#5e00ff'; + --tw-ring-color: rgb(var(--color-primary)); + border-color: rgb(var(--color-primary));Or if this color is defined in your Tailwind config:
- --tw-ring-color: '#5e00ff'; - border-color: '#5e00ff'; + --tw-ring-color: theme('colors.primary.DEFAULT'); + border-color: theme('colors.primary.DEFAULT');
7-22: Consider including additional input types in the reset.The form reset is missing some modern input types that might be used in the app.
[type='text'], input:where(:not([type])), [type='email'], [type='url'], [type='password'], [type='number'], [type='date'], [type='datetime-local'], [type='month'], [type='search'], [type='tel'], [type='time'], [type='week'], +[type='color'], +[type='file'], +[type='range'], [multiple], textarea, select {src/apps/pulse/components/Esc.tsx (1)
7-7: Use consistent styling approach and simplify onClick handler.Similar to the
Closecomponent, this mixes inline styles with the app's Tailwind approach. Also, the arrow function wrapper in onClick is unnecessary.- <button style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}} onClick={() => {props.closePreview()}}> + <button className="bg-black rounded-[10px] w-10 h-10" onClick={props.closePreview}>src/apps/pulse/components/AppWrapper.tsx (1)
1-1: Remove unused import.The
useRefimport is not used in this component.-import { SetStateAction, useRef, useState } from "react"; +import { SetStateAction, useState } from "react";src/apps/pulse/hooks/useExpressIntent.ts (3)
14-14: Remove redundant optional chaining.The optional chaining operator is redundant here since
intentSdkis already checked for truthiness in the condition above.- intentSdk?.expressIntent(userIntent).then((response) => { + intentSdk.expressIntent(userIntent).then((response) => {
9-9: Consider improving the refetch mechanism.The current refetch mechanism using a boolean toggle works but could be more intuitive. Consider using a function-based approach or an incrementing counter.
- const [refetch, setRefetch] = useState(false); + const [refetchTrigger, setRefetchTrigger] = useState(0); + const refetch = useCallback(() => setRefetchTrigger(prev => prev + 1), []); useEffect(() => { // ... existing logic - }, [userIntent, intentSdk, refetch]); + }, [userIntent, intentSdk, refetchTrigger]); return { isLoading, expressIntentResponse, - refetch, - setRefetch + refetch }Also applies to: 31-33
17-21: Consider exposing errors to hook consumers.Currently, errors are only logged to the console. Consider exposing the error state to allow consumers to handle errors appropriately.
const [isLoading, setIsLoading] = useState<boolean>(false); const [expressIntentResponse, setExpressIntentResponse] = useState<ExpressIntentResponse | null>(null); + const [error, setError] = useState<Error | null>(null); }).catch((error) => { console.error("Error in expressIntent: ", error); setIsLoading(false); setExpressIntentResponse(null); + setError(error); }) return { isLoading, expressIntentResponse, + error, refetch, setRefetch }src/apps/pulse/components/PayingToken.tsx (1)
9-46: Consider consistent styling approach.The component mixes inline styles with Tailwind CSS classes. For better maintainability, consider using a consistent approach throughout the component.
- <div - className="flex justify-between" - style={{marginTop: 10, marginBottom: 10}} - > + <div className="flex justify-between my-2.5"> <div className="flex items-center"> - <div style={{position: "relative", display: "inline-block" }}> + <div className="relative inline-block"> <img src={props.payingToken.logo} alt="Main" - style={{ - width: 32, - height: 32, - borderRadius: 50, - marginLeft: 5, - marginRight: 5 - }} + className="w-8 h-8 rounded-full mx-1.5" /> <img src={getLogoForChainId(props.payingToken.chainId)} - style={{position: "absolute", - bottom: "-1px", - right: "2px", - width: 12, - height: 12, - borderRadius: "50%", - }} + className="absolute -bottom-px right-0.5 w-3 h-3 rounded-full" /> </div> - <div style={{marginLeft: 5}}> - <div style={{fontSize: 13}}>{props.payingToken.name}</div> - <div style={{fontSize: 13, color: "grey"}}>{props.payingToken.actualBal} {props.payingToken.symbol}</div> + <div className="ml-1.5"> + <div className="text-xs">{props.payingToken.name}</div> + <div className="text-xs text-gray-500">{props.payingToken.actualBal} {props.payingToken.symbol}</div> </div> </div> - <div className="flex flex-col justify-center" style={{marginRight: 10}}> - <div className="flex" style={{fontSize: 13, textAlign: "right"}}>{props.payingToken.totalRaw}</div> - <div className="flex justify-end" style={{fontSize: 12, color: "grey", textAlign: "right"}}>${props.payingToken.totalUsd.toFixed(2)}</div> + <div className="flex flex-col justify-center mr-2.5"> + <div className="text-xs text-right">{props.payingToken.totalRaw}</div> + <div className="text-xs text-gray-500 text-right">${props.payingToken.totalUsd.toFixed(2)}</div> </div> </div>src/apps/pulse/types/tokens.ts (1)
1-32: Consider adding JSDoc comments for better documentation.Adding JSDoc comments would improve type documentation and developer experience.
+/** + * Represents a parsed token with market data + */ export type ParsedToken = { name: string; symbol: string; marketCap: number; liquidity: number; volume: number; logo: string; price: number; priceChange: number; blockchain: string; } +/** + * Represents a token selected by the user + */ export type SelectedToken = { name: string; symbol: string; usdValue: string; logo: string; dailyPriceChange: number; chainId: number; decimals: number; address: string; } +/** + * Represents a token used for payment/transaction + */ export type PayingToken = { name: string; symbol: string; logo: string; actualBal: string; totalUsd: number; totalRaw: string; chainId: number; }src/apps/pulse/components/Sell.tsx (1)
12-39: Extract hardcoded styling values to constants.Consider extracting hardcoded styling values to constants for better maintainability and consistency.
+const STYLES = { + CONTAINER: { margin: 10, backgroundColor: "black", width: 422, height: 100, borderRadius: 10 }, + AMOUNT_BUTTON: { backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10 }, + BUTTON_INNER: { backgroundColor: "#121116", borderRadius: 10, margin: 2, color: "grey" }, + SELL_BUTTON: { margin: 10, width: 422, height: 50, borderRadius: 10 } +}; export default function Sell(props: Props) { return ( <> <div className="flex" - style={{margin: 10, backgroundColor: "black", width: 422, height: 100, borderRadius: 10}} + style={STYLES.CONTAINER} > </div> {/* amounts */} <div className="flex"> {["$10", "$20", "$50", "$100", "MAX"].map((item, index) => { return ( - <div key={index} className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}> - <button className="flex-1 items-center justify-center" style={{backgroundColor: "#121116", borderRadius: 10, margin: 2, color: "grey"}}> + <div key={index} className="flex" style={STYLES.AMOUNT_BUTTON}> + <button className="flex-1 items-center justify-center" style={STYLES.BUTTON_INNER}> {item} </button> </div> ) })} </div> {/* buy/sell button */} <div className="flex bg-deep_purple-A700" - style={{margin: 10, width: 422, height: 50, borderRadius: 10}} + style={STYLES.SELL_BUTTON} > <button className="flex-1 items-center justify-center"> Sell </button> </div> </> ); }src/apps/pulse/hooks/useIntentSdk.ts (1)
32-32: Improve type assertion for better type safety.The
as anytype assertion should be avoided. Consider using proper typing or a more specific type assertion.- const sdk = new IntentSdk(walletClient as any, options); + const sdk = new IntentSdk(walletClient, options);If the type mismatch is unavoidable, consider adding a comment explaining why the type assertion is necessary.
src/apps/pulse/utils/parseSearchData.ts (2)
43-45: Implement the parseTokenData function.The
parseTokenDatafunction is declared but not implemented. Either implement it or remove the declaration if it's not needed yet.-export function parseTokenData(token: PairResponse) { - -} +export function parseTokenData(token: PairResponse): Asset[] { + // TODO: Implement token data parsing + return []; +}Do you want me to help implement this function based on the
PairResponsestructure?
55-60: Remove unnecessary continue statements.The continue statements are unnecessary as they're at the end of the loop iteration.
for(const item of searchData) { if(item.type === "asset") { assets.push(...parseAssetData(item as TokenAssetResponse)); } else if (item.type === "token"){ // parseTokenData(item as PairResponse); - continue; - } else { - continue; } }src/apps/pulse/components/HomeScreen.tsx (2)
44-85: Extract inline styles to improve maintainabilityThe component has extensive inline styles with hardcoded values. Consider extracting these to a theme configuration or CSS modules for better maintainability and consistency across the Pulse app.
Example approach:
const styles = { container: { backgroundColor: 'black', minHeight: '100vh', }, searchButton: { border: '2px solid #1E1D24', width: 446, height: 40, backgroundColor: "#121116", borderRadius: 10 }, mainPanel: { border: '2px solid #1E1D24', width: 446, height: 264, backgroundColor: "#121116", borderRadius: 10, marginTop: 40, } // ... other styles };
90-110: Simplify toggle button styling logicThe Buy/Sell toggle buttons have repetitive conditional styling. Consider extracting this to a helper function for cleaner code.
+const getToggleStyle = (isActive: boolean) => ({ + backgroundColor: isActive ? "#121116" : "black", + borderRadius: 10, + margin: 4, + color: isActive ? undefined : "grey" +}); <button className="flex-1" - style={ - props.isBuy ? - {backgroundColor: "#121116", borderRadius: 10, margin: 4}: - {backgroundColor: "black", borderRadius: 10, margin: 4, color: "grey"} - } + style={getToggleStyle(props.isBuy)} onClick={() => props.setIsBuy(true)} >src/apps/pulse/components/IntentTracker.tsx (2)
84-84: Add alt attribute for accessibilityThe img tag is missing an alt attribute, which is important for accessibility.
- <img src={props.token.logo} style={{borderRadius: 50, height: 32, width: 32, marginLeft: 10}}/> + <img src={props.token.logo} alt={`${props.token.symbol} logo`} style={{borderRadius: 50, height: 32, width: 32, marginLeft: 10}}/>
68-68: Extract polling interval to a constantThe polling interval is hardcoded. Consider making it configurable or at least extracting to a named constant.
+const POLLING_INTERVAL_MS = 15000; // 15 seconds if (!isCancelled) { - setTimeout(poll, 15000); + setTimeout(poll, POLLING_INTERVAL_MS); }src/apps/pulse/components/Buy.tsx (2)
23-23: Use exponentiation operator instead of Math.powModern JavaScript prefers the
**operator overMath.powfor better readability.- return BigInt(Number(value.toFixed(4)) * Math.pow(10, decimals)); + return BigInt(Number(value.toFixed(4)) * (10 ** decimals));
26-71: Improve getDispensableAssets function robustnessThe function has a TODO about supporting multiple tokens and uses complex nested logic. Consider refactoring for clarity and implementing the multi-token support mentioned in the TODO.
The function could benefit from:
- Better error handling for edge cases
- Extracting the token matching logic
- Implementing multi-token support as mentioned in the TODO
Would you like me to help refactor this function to support multiple tokens from different chains?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pulse/icon.pngis excluded by!**/*.png
📒 Files selected for processing (25)
README.md(1 hunks)config-overrides.js(1 hunks)package.json(2 hunks)src/apps/pulse/components/AppWrapper.tsx(1 hunks)src/apps/pulse/components/Buy.tsx(1 hunks)src/apps/pulse/components/Close.tsx(1 hunks)src/apps/pulse/components/Esc.tsx(1 hunks)src/apps/pulse/components/HomeScreen.tsx(1 hunks)src/apps/pulse/components/IntentTracker.tsx(1 hunks)src/apps/pulse/components/PayingToken.tsx(1 hunks)src/apps/pulse/components/PreviewBuy.tsx(1 hunks)src/apps/pulse/components/Refresh.tsx(1 hunks)src/apps/pulse/components/Search.tsx(1 hunks)src/apps/pulse/components/Sell.tsx(1 hunks)src/apps/pulse/constants/tokens.ts(1 hunks)src/apps/pulse/hooks/useExpressIntent.ts(1 hunks)src/apps/pulse/hooks/useIntentSdk.ts(1 hunks)src/apps/pulse/hooks/useTokenSearch.ts(1 hunks)src/apps/pulse/index.tsx(1 hunks)src/apps/pulse/manifest.json(1 hunks)src/apps/pulse/styles/tailwindPulse.css(1 hunks)src/apps/pulse/tailwind.pulse.config.js(1 hunks)src/apps/pulse/types/tokens.ts(1 hunks)src/apps/pulse/utils/constants.ts(1 hunks)src/apps/pulse/utils/parseSearchData.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/apps/pulse/index.tsx (1)
src/apps/pulse/components/AppWrapper.tsx (1)
AppWrapper(5-28)
src/apps/pulse/types/tokens.ts (1)
src/apps/pulse/components/PayingToken.tsx (1)
PayingToken(7-48)
src/apps/pulse/components/PreviewBuy.tsx (6)
src/apps/pulse/components/PayingToken.tsx (1)
PayingToken(7-48)src/apps/pulse/hooks/useIntentSdk.ts (1)
useIntentSdk(7-39)src/apps/pulse/components/IntentTracker.tsx (1)
IntentTracker(48-155)src/apps/pulse/components/Refresh.tsx (1)
Refresh(1-19)src/apps/pulse/components/Esc.tsx (1)
Esc(5-23)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)
src/apps/pulse/components/PayingToken.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(158-192)
src/apps/pulse/components/HomeScreen.tsx (4)
src/apps/pulse/components/PayingToken.tsx (1)
PayingToken(7-48)src/apps/pulse/components/PreviewBuy.tsx (1)
PreviewBuy(18-221)src/apps/pulse/components/Buy.tsx (1)
Buy(73-364)src/apps/pulse/components/Sell.tsx (1)
Sell(9-42)
src/apps/pulse/components/Search.tsx (5)
src/apps/pulse/hooks/useTokenSearch.ts (1)
useTokenSearch(4-36)src/apps/pulse/utils/parseSearchData.ts (2)
parseSearchData(48-64)Asset(5-17)src/apps/pulse/utils/constants.ts (1)
MOBULA_CHAIN_NAMES_TO_CHAIN_ID(11-19)src/apps/pulse/components/Close.tsx (1)
Close(1-9)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)
src/apps/pulse/components/Buy.tsx (5)
src/types/api.ts (2)
WalletPortfolioMobulaResponse(742-744)PortfolioData(731-740)src/apps/pulse/components/PayingToken.tsx (1)
PayingToken(7-48)src/apps/pulse/constants/tokens.ts (1)
STABLE_CURRENCIES(1-6)src/apps/pulse/hooks/useIntentSdk.ts (1)
useIntentSdk(7-39)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)
🪛 LanguageTool
README.md
[uncategorized] ~3-~3: Possible missing comma found.
Context: ... care of the infrastructure and account management so you don't need to. 🛠️ **Here to bu...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
README.md
3-3: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
🪛 Biome (1.9.4)
src/apps/pulse/components/Sell.tsx
[error] 22-22: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/PreviewBuy.tsx
[error] 176-176: Prefer constants from the standard library.
Unsafe fix: Use Math.LOG2E instead.
(lint/suspicious/noApproximativeNumericConstant)
[error] 108-108: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/utils/parseSearchData.ts
[error] 57-57: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 59-59: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
src/apps/pulse/components/Search.tsx
[error] 110-111: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 129-130: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Buy.tsx
[error] 321-321: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
🔇 Additional comments (10)
README.md (1)
3-3: LGTM! Typo fix improves documentation quality.The correction from "infrstructure" to "infrastructure" is appropriate and enhances the professional appearance of the documentation.
package.json (2)
79-79: LGTM! Appropriate dependency for loading spinners.The
react-loader-spinnerpackage is a well-established library for loading indicators, which aligns with the UI requirements of the Pulse app.
22-22: Verify the alpha version dependency for production readiness.The
@etherspot/intent-sdkpackage is using an alpha version (^1.0.0-alpha.2), which may not be suitable for production environments due to potential instability and breaking changes.What is the current stable version of @etherspot/intent-sdk and are there any known issues with version 1.0.0-alpha.2?src/apps/pulse/utils/constants.ts (1)
1-19: LGTM! Well-structured constants for multi-chain support.The constants are properly organized with:
- Clear naming convention
- Accurate chain ID mappings
- Proper TypeScript type annotations
- Good separation of concerns
The chain IDs appear correct for the listed networks (Ethereum: 1, Polygon: 137, Base: 8453, etc.).
config-overrides.js (1)
19-21: Verify SDK dependencies for disabled core modulesSetting
net,tls, andchild_processtofalsereduces bundle size, but any runtime import from@etherspot/intent-sdk(or future deps) will now throw “Module not found”.
Run a quickrg "require('net'|\"net\")" node_modules/@etherspotbefore merging.src/apps/pulse/components/AppWrapper.tsx (1)
8-9: ```shell
#!/bin/bashSearch for SelectedToken type definition and imports across the repository
rg -n "interface SelectedToken" .
rg -n "type SelectedToken" .
rg -n "import.*SelectedToken" .</details> <details> <summary>src/apps/pulse/components/PayingToken.tsx (1)</summary> `1-1`: ```shell #!/bin/bash # Locate definition of the PayingToken type rg "export (type|interface) PayingToken" --type tssrc/apps/pulse/utils/parseSearchData.ts (1)
49-63: Clarify the purpose of the markets array.The
marketsarray is declared but never populated. Either remove it or clarify its intended purpose.Is the
marketsarray intended to store parsed token data? If so, update the logic to populate it whenparseTokenDatais implemented.for(const item of searchData) { if(item.type === "asset") { assets.push(...parseAssetData(item as TokenAssetResponse)); } else if (item.type === "token"){ - // parseTokenData(item as PairResponse); + markets.push(...parseTokenData(item as PairResponse)); } }src/apps/pulse/components/PreviewBuy.tsx (1)
84-89: Validate data before passing to IntentTrackerThe component passes data with non-null assertions which could cause runtime errors if the data is unexpectedly null.
- bidHash={props.expressIntentResponse?.bids[0].bidHash!} - token={props.buyToken!} + bidHash={props.expressIntentResponse?.bids[0].bidHash || ""} + token={props.buyToken || {} as SelectedToken}Consider adding proper validation before rendering the IntentTracker component.
src/apps/pulse/components/IntentTracker.tsx (1)
15-25: Handle edge case for unknown statusThe function returns -1 for unknown status, but this edge case isn't handled in the UI logic where the index is used for comparisons.
Consider throwing an error or handling the -1 case explicitly in the UI to prevent unexpected behavior.
const getStatusIndex = (status: "PENDING" | "SHORTLISTING_INITIATED" | "SHORTLISTED" | "EXECUTED") => { if(status === "PENDING") return 0; if(status === "SHORTLISTING_INITIATED") return 1; if(status === "SHORTLISTED") return 2; if(status === "EXECUTED") return 3; - return -1; + console.error(`Unknown bid status: ${status}`); + return 0; // Default to PENDING }
| export default function Refresh() { | ||
| return ( | ||
| <button style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}}> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 20 20" | ||
| fill="grey" | ||
| className="size-5" | ||
| style={{backgroundColor: "#121116", borderRadius: 10, margin: 2}} | ||
| > | ||
| <path | ||
| fillRule="evenodd" | ||
| d="M15.312 11.424a5.5 5.5 0 0 1-9.201 2.466l-.312-.311h2.433a.75.75 0 0 0 0-1.5H3.989a.75.75 0 0 0-.75.75v4.242a.75.75 0 0 0 1.5 0v-2.43l.31.31a7 7 0 0 0 11.712-3.138.75.75 0 0 0-1.449-.39Zm1.23-3.723a.75.75 0 0 0 .219-.53V2.929a.75.75 0 0 0-1.5 0V5.36l-.31-.31A7 7 0 0 0 3.239 8.188a.75.75 0 1 0 1.448.389A5.5 5.5 0 0 1 13.89 6.11l.311.31h-2.432a.75.75 0 0 0 0 1.5h4.243a.75.75 0 0 0 .53-.219Z" | ||
| clipRule="evenodd" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add functionality and improve accessibility.
The component currently lacks essential functionality and accessibility features:
- Missing onClick handler: A refresh button should have click functionality
- No accessibility attributes: Missing
aria-labelandroleattributes - Inline styles: Consider using CSS classes for better maintainability
-export default function Refresh() {
+export default function Refresh({ onClick }: { onClick?: () => void }) {
return (
- <button style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}}>
+ <button
+ onClick={onClick}
+ aria-label="Refresh"
+ className="bg-black rounded-[10px] w-10 h-10"
+ style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}}
+ >
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20"
fill="grey"
className="size-5"
style={{backgroundColor: "#121116", borderRadius: 10, margin: 2}}
+ aria-hidden="true"
>📝 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.
| export default function Refresh() { | |
| return ( | |
| <button style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}}> | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| viewBox="0 0 20 20" | |
| fill="grey" | |
| className="size-5" | |
| style={{backgroundColor: "#121116", borderRadius: 10, margin: 2}} | |
| > | |
| <path | |
| fillRule="evenodd" | |
| d="M15.312 11.424a5.5 5.5 0 0 1-9.201 2.466l-.312-.311h2.433a.75.75 0 0 0 0-1.5H3.989a.75.75 0 0 0-.75.75v4.242a.75.75 0 0 0 1.5 0v-2.43l.31.31a7 7 0 0 0 11.712-3.138.75.75 0 0 0-1.449-.39Zm1.23-3.723a.75.75 0 0 0 .219-.53V2.929a.75.75 0 0 0-1.5 0V5.36l-.31-.31A7 7 0 0 0 3.239 8.188a.75.75 0 1 0 1.448.389A5.5 5.5 0 0 1 13.89 6.11l.311.31h-2.432a.75.75 0 0 0 0 1.5h4.243a.75.75 0 0 0 .53-.219Z" | |
| clipRule="evenodd" | |
| /> | |
| </svg> | |
| </button> | |
| ) | |
| } | |
| export default function Refresh({ onClick }: { onClick?: () => void }) { | |
| return ( | |
| <button | |
| onClick={onClick} | |
| aria-label="Refresh" | |
| className="bg-black rounded-[10px] w-10 h-10" | |
| style={{backgroundColor: "black", borderRadius: 10, width: 40, height: 40}} | |
| > | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| viewBox="0 0 20 20" | |
| fill="grey" | |
| className="size-5" | |
| style={{backgroundColor: "#121116", borderRadius: 10, margin: 2}} | |
| aria-hidden="true" | |
| > | |
| <path | |
| fillRule="evenodd" | |
| d="M15.312 11.424a5.5 5.5 0 0 1-9.201 2.466l-.312-.311h2.433a.75.75 0 0 0 0-1.5H3.989a.75.75 0 0 0-.75.75v4.242a.75.75 0 0 0 1.5 0v-2.43l.31.31a7 7 0 0 0 11.712-3.138.75.75 0 0 0-1.449-.39Zm1.23-3.723a.75.75 0 0 0 .219-.53V2.929a.75.75 0 0 0-1.5 0V5.36l-.31-.31A7 7 0 0 0 3.239 8.188a.75.75 0 1 0 1.448.389A5.5 5.5 0 0 1 13.89 6.11l.311.31h-2.432a.75.75 0 0 0 0 1.5h4.243a.75.75 0 0 0 .53-.219Z" | |
| clipRule="evenodd" | |
| /> | |
| </svg> | |
| </button> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Refresh.tsx lines 1 to 19, the Refresh button lacks
an onClick handler to provide refresh functionality, and it is missing
accessibility attributes such as aria-label and role to improve screen reader
support. Additionally, inline styles are used which reduce maintainability. To
fix this, add a prop or internal onClick handler to handle refresh actions,
include appropriate aria-label and role attributes on the button element, and
refactor the inline styles into CSS classes for better styling management.
| useEffect(() => { | ||
| if(props.isBuy) { | ||
| const handler = setTimeout(() => { | ||
| setDebouncedSearchText(searchText); | ||
| }, 1000); | ||
| return () => clearTimeout(handler); | ||
| } | ||
| }, [searchText]); |
There was a problem hiding this comment.
❓ Verification inconclusive
Fix conditional debouncing logic and missing dependency.
There are several issues with the debouncing implementation:
- Conditional debouncing bug: When
isBuyis false,debouncedSearchTextis never updated, but the query still uses it - Missing dependency:
props.isBuyshould be in the useEffect dependency array
useEffect(() => {
- if(props.isBuy) {
const handler = setTimeout(() => {
setDebouncedSearchText(searchText);
}, 1000);
return () => clearTimeout(handler);
- }
- }, [searchText]);
+ }, [searchText, props.isBuy]);Is the conditional debouncing intentional? If search should work differently for buy vs sell operations, please clarify the expected behavior.
Fix conditional debouncing logic and missing dependency.
There are several issues with the debouncing implementation:
- Conditional debouncing bug: When
isBuyis false,debouncedSearchTextis never updated, but the query still uses it - Missing dependency:
props.isBuyshould be in the useEffect dependency array
useEffect(() => {
- if(props.isBuy) {
const handler = setTimeout(() => {
setDebouncedSearchText(searchText);
}, 1000);
return () => clearTimeout(handler);
- }
- }, [searchText]);
+ }, [searchText, props.isBuy]);Is the conditional debouncing intentional? If search should work differently for buy vs sell operations, please clarify the expected behavior.
🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useTokenSearch.ts around lines 8 to 15, the useEffect
hook conditionally debounces updating debouncedSearchText only when props.isBuy
is true, causing debouncedSearchText to never update when isBuy is false, which
breaks the query logic. Also, props.isBuy is missing from the dependency array.
To fix this, remove the conditional inside useEffect so debouncedSearchText
updates on every searchText change regardless of isBuy, and add props.isBuy to
the dependency array to ensure the effect runs correctly when isBuy changes.
| export const STABLE_CURRENCIES = [ | ||
| {chainId: 10, address: "0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85"}, | ||
| {chainId: 137, address: "0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359"}, | ||
| {chainId: 8453, address: "0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913"}, | ||
| {chainId: 42161, address: "0xaf88d065e77c8cC2239327C5EDb3A432268e5831"}, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit typing + immutability for STABLE_CURRENCIES
Relying on an implicit any[] array means any object can be pushed later.
Guard this by defining a StableCurrency type and exporting a frozen ReadonlyArray.
+export type StableCurrency = Readonly<{
+ chainId: number;
+ address: `0x${string}`;
+}>;
-export const STABLE_CURRENCIES = [
- {chainId: 10, address: "0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85"},
- ...
-];
+export const STABLE_CURRENCIES: ReadonlyArray<StableCurrency> = Object.freeze([
+ { chainId: 10, address: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85' },
+ ...
+]);This prevents accidental mutations and gives downstream code proper IntelliSense.
📝 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.
| export const STABLE_CURRENCIES = [ | |
| {chainId: 10, address: "0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85"}, | |
| {chainId: 137, address: "0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359"}, | |
| {chainId: 8453, address: "0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913"}, | |
| {chainId: 42161, address: "0xaf88d065e77c8cC2239327C5EDb3A432268e5831"}, | |
| ]; | |
| // src/apps/pulse/constants/tokens.ts | |
| export type StableCurrency = Readonly<{ | |
| chainId: number; | |
| address: `0x${string}`; | |
| }>; | |
| export const STABLE_CURRENCIES: ReadonlyArray<StableCurrency> = Object.freeze([ | |
| { chainId: 10, address: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85' }, | |
| { chainId: 137, address: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359' }, | |
| { chainId: 8453, address: '0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913' }, | |
| { chainId: 42161,address: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831' }, | |
| ]); |
🤖 Prompt for AI Agents
In src/apps/pulse/constants/tokens.ts lines 1 to 6, the STABLE_CURRENCIES array
lacks explicit typing and immutability, which allows accidental mutations and
weak IntelliSense. Define a StableCurrency type describing the shape of each
object with chainId and address properties, then declare STABLE_CURRENCIES as a
ReadonlyArray of StableCurrency. Finally, export the array as a frozen object
using Object.freeze to prevent any modifications.
| content: [ | ||
| './src/apps/pillarx-app/**/**/*.{js,ts,jsx,tsx,html,mdx}', | ||
| './src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}', | ||
| ], |
There was a problem hiding this comment.
Pulse files are excluded from the purge list
content only targets pillarx-app paths.
Classes used in src/apps/pulse/** will be tree-shaken, leaving blank styling in production.
-content: [
- './src/apps/pillarx-app/**/**/*.{js,ts,jsx,tsx,html,mdx}',
- './src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}',
-],
+content: [
+ './src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}',
+ './src/apps/pulse/**/*.{js,ts,jsx,tsx,html,mdx}',
+],📝 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.
| content: [ | |
| './src/apps/pillarx-app/**/**/*.{js,ts,jsx,tsx,html,mdx}', | |
| './src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}', | |
| ], | |
| content: [ | |
| './src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}', | |
| './src/apps/pulse/**/*.{js,ts,jsx,tsx,html,mdx}', | |
| ], |
🤖 Prompt for AI Agents
In src/apps/pulse/tailwind.pulse.config.js around lines 4 to 7, the content
array only includes paths for pillarx-app, excluding pulse app files. Update the
content array to also include all relevant file paths under src/apps/pulse/**
with appropriate glob patterns to ensure pulse files are included in the purge
list and their styles are preserved in production.
src/apps/pulse/components/Buy.tsx
Outdated
| deadline: BigInt(Math.floor(Date.now() / 1000)) + BigInt(60), | ||
| desiredAssets: [{ | ||
| asset: props.token.address as Hex, | ||
| chainId: BigInt(props.token.chainId), | ||
| value: getDesiredAssetValue(input, props.token.decimals, props.token.usdValue) | ||
| }], | ||
| dispensableAssets, | ||
| maxGas: BigInt(6000000), | ||
| permittedChains, | ||
| slippagePercentage: 5, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic numbers to named constants
The intent configuration uses several magic numbers that should be extracted to named constants for better maintainability.
const INTENT_CONFIG = {
DEADLINE_SECONDS: 60,
MAX_GAS: 6000000,
DEFAULT_SLIPPAGE_PERCENTAGE: 5
};
// Then use:
deadline: BigInt(Math.floor(Date.now() / 1000)) + BigInt(INTENT_CONFIG.DEADLINE_SECONDS),
maxGas: BigInt(INTENT_CONFIG.MAX_GAS),
slippagePercentage: INTENT_CONFIG.DEFAULT_SLIPPAGE_PERCENTAGE,🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy.tsx around lines 94 to 103, the code uses magic
numbers for deadline seconds, max gas, and slippage percentage directly in the
intent configuration. To fix this, define named constants for these values
(e.g., DEADLINE_SECONDS = 60, MAX_GAS = 6000000, DEFAULT_SLIPPAGE_PERCENTAGE =
5) in a suitable place and replace the hardcoded numbers with these constants to
improve maintainability and readability.
src/apps/pulse/components/Buy.tsx
Outdated
| {["10", "20", "50", "100", "MAX"].map(item => { | ||
| return ( | ||
| <div className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}> | ||
| <button | ||
| className="flex-1 items-center justify-center" | ||
| style={{backgroundColor: "#121116", borderRadius: 10, margin: 2, color: "grey"}} | ||
| onClick={() => { | ||
| if(item === "MAX") { | ||
| setUsdAmount(props.walletPortfolioData?.result.data.total_wallet_balance.toFixed(2) ?? "0.00") | ||
| } else { | ||
| setUsdAmount(parseFloat(item).toFixed(2)) | ||
| } | ||
| }} | ||
| > | ||
| ${item} | ||
| </button> | ||
| </div> | ||
| ) | ||
| })} |
There was a problem hiding this comment.
Add key prop to amount buttons
React requires key props for list items.
{["10", "20", "50", "100", "MAX"].map(item => {
return (
- <div className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}>
+ <div key={item} className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}>📝 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.
| {["10", "20", "50", "100", "MAX"].map(item => { | |
| return ( | |
| <div className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}> | |
| <button | |
| className="flex-1 items-center justify-center" | |
| style={{backgroundColor: "#121116", borderRadius: 10, margin: 2, color: "grey"}} | |
| onClick={() => { | |
| if(item === "MAX") { | |
| setUsdAmount(props.walletPortfolioData?.result.data.total_wallet_balance.toFixed(2) ?? "0.00") | |
| } else { | |
| setUsdAmount(parseFloat(item).toFixed(2)) | |
| } | |
| }} | |
| > | |
| ${item} | |
| </button> | |
| </div> | |
| ) | |
| })} | |
| {["10", "20", "50", "100", "MAX"].map(item => { | |
| return ( | |
| <div | |
| key={item} | |
| className="flex" | |
| style={{ | |
| backgroundColor: "black", | |
| marginLeft: 10, | |
| width: 75, | |
| height: 30, | |
| borderRadius: 10 | |
| }} | |
| > | |
| <button | |
| className="flex-1 items-center justify-center" | |
| style={{ | |
| backgroundColor: "#121116", | |
| borderRadius: 10, | |
| margin: 2, | |
| color: "grey" | |
| }} | |
| onClick={() => { | |
| if (item === "MAX") { | |
| setUsdAmount( | |
| props.walletPortfolioData?.result.data | |
| .total_wallet_balance.toFixed(2) ?? "0.00" | |
| ); | |
| } else { | |
| setUsdAmount(parseFloat(item).toFixed(2)); | |
| } | |
| }} | |
| > | |
| ${item} | |
| </button> | |
| </div> | |
| ); | |
| })} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 321-321: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy.tsx around lines 319 to 337, the map function
rendering the amount buttons is missing a key prop on the root element inside
the loop. Add a unique key prop to the outer div element, for example using the
item value, to satisfy React's requirement for stable keys in lists and prevent
rendering issues.
| return result; | ||
| } | ||
|
|
||
| export function parseTokenData(token: PairResponse) { |
There was a problem hiding this comment.
Empty function and unused
| if(item.type === "asset") { | ||
| assets.push(...parseAssetData(item as TokenAssetResponse)); | ||
| } else if (item.type === "token"){ | ||
| // parseTokenData(item as PairResponse); |
There was a problem hiding this comment.
Is this supposed to be commented?
src/apps/pulse/utils/constants.ts
Outdated
| "Optimistic" | ||
| ]; | ||
|
|
||
| export const MOBULA_CHAIN_NAMES_TO_CHAIN_ID: {[key: string]: number} = { |
There was a problem hiding this comment.
Just one thing to consider is that we have a function that converts mobula chain names into chain ids in the file called tokensData.ts
src/apps/pulse/types/tokens.ts
Outdated
| @@ -0,0 +1,32 @@ | |||
| type ParsedToken = { | |||
There was a problem hiding this comment.
I did not see this type used anywhere in the app. Also as Coderabbit AI mentioned, none of those type are exported. Make sure to export them
| @@ -0,0 +1,16 @@ | |||
| export const STABLE_CURRENCIES = [ | |||
There was a problem hiding this comment.
I would add at least as a comment the name and symbol of those stable currencies for any future devs reference
src/apps/pulse/constants/tokens.ts
Outdated
| {chainId: 42161, address: "0xaf88d065e77c8cC2239327C5EDb3A432268e5831"}, | ||
| ]; | ||
|
|
||
| export const CHAIN_LOGOS : {[key: string]: string} = { |
There was a problem hiding this comment.
In the file called blockchain.ts, there is a function called getLogoForChainId that you can used to add the logo for each chain that we support
There was a problem hiding this comment.
Is that the right icon for Pulse?
src/apps/pulse/components/Search.tsx
Outdated
| address: item.contract, | ||
| }); | ||
| } else { | ||
| props.setSellToken({ |
There was a problem hiding this comment.
Why are those values hardcoded?
There was a problem hiding this comment.
the sell flow isn't completed yet, just to avoid typescript compilation errors, i have hardcoded a token.
this will be taken care once sell flow is completed
src/apps/pulse/components/Search.tsx
Outdated
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| placeholder="" |
There was a problem hiding this comment.
if there is not placeholder, you can remove this param completely
src/apps/pulse/components/Search.tsx
Outdated
| /> | ||
| { | ||
| searchText.length > 0 ? | ||
| isFetching ? |
There was a problem hiding this comment.
I do understand this ternary operator here: do you mean is searchText.length > 0 AND if isFetching, then it should show spinning? In that case I would do this:
searchText.length > 0 && isFetching ? ...
Instead of:
searchText.length > 0 ? isFetching ? ...
| @@ -0,0 +1,168 @@ | |||
| import { Dispatch, RefObject, SetStateAction, useRef, useState } from "react"; | |||
There was a problem hiding this comment.
RefObject and useRef are not used in this file
| import Sell from "./Sell"; | ||
| import { useWalletAddress } from "@etherspot/transaction-kit"; | ||
| import { useGetWalletPortfolioQuery } from "../../../services/pillarXApiWalletPortfolio"; | ||
| import useIntentSdk from "../hooks/useIntentSdk"; |
There was a problem hiding this comment.
useIntentSdk is not used in this file
| import PreviewBuy from "./PreviewBuy"; | ||
| import { ExpressIntentResponse } from "@etherspot/intent-sdk/dist/cjs/sdk/types/user-intent-types"; | ||
|
|
||
| interface Props { |
There was a problem hiding this comment.
I would give this another name such as HomeScreenProps instead of just Props
|
|
||
| const { | ||
| data: walletPortfolioData, | ||
| isLoading: isWalletPortfolioDataLoading, |
There was a problem hiding this comment.
isLoading, isFetching and refetch are not being used in this file, however how do you make sure you handle any errors or no data coming from this endpoint?
src/apps/pulse/components/Buy.tsx
Outdated
| } | ||
| </div> | ||
| <div className="flex" style={{float: "right"}}> | ||
| <svg width="18" height="12" viewBox="0 0 18 12" fill="none" xmlns="http://www.w3.org/2000/svg"> |
There was a problem hiding this comment.
I would advise to put svg into there own component (you can create an "images" folder, just to improve the visibility in this code
| /** @type {import('tailwindcss').Config} */ | ||
| // eslint-disable-next-line no-undef | ||
| module.exports = { | ||
| content: [ |
There was a problem hiding this comment.
if you want tailwind to apply the styles below in your app, you should have:
content: [
'./src/apps/pulse/**/**/*.{js,ts,jsx,tsx,html,mdx}',
'./src/apps/pulse/**/*.{js,ts,jsx,tsx,html,mdx}',
],
Because currently it shows for the pillarx-app not for pulse:
content: [
'./src/apps/pillarx-app/**/**/*.{js,ts,jsx,tsx,html,mdx}',
'./src/apps/pillarx-app/**/*.{js,ts,jsx,tsx,html,mdx}',
],
There was a problem hiding this comment.
Also, it is important that you add this app into the .eslintrc.json file in the list like so:
"!src/apps/pulse/",
"!src/apps/pulse/**",
This is important for code quality! Once you add these lines into the .eslintrc.json, make sure to CMD + SHIT + P ---> ESlint: Restart ESlint server, then look back at your files, and you will notice a lot of linting fixes to do!
There was a problem hiding this comment.
i don't think i am using any of the tailwind styles, but will edit it so that it can be usable for future
Deploying pulse with
|
| Latest commit: |
c67305b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f898ffde.pulse-19m.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
src/apps/pulse/components/Buy.tsx (7)
46-46: Use strict equality operatorUse
===instead of==to avoid type coercion issues.- (token) => token.address.toLowerCase() == tokenItem.address.toLowerCase() && token.chainId == tokenItem.chainId + (token) => token.address.toLowerCase() === tokenItem.address.toLowerCase() && token.chainId === tokenItem.chainId
87-87: Fix syntax error in conditionThere's a misplaced parenthesis causing a syntax error in the condition.
- if(Number(parseFloat(input) > 0) && props.token && accountAddress && intentSdk) { + if(Number(parseFloat(input)) > 0 && props.token && accountAddress && intentSdk) {
89-89: Use strict equality operatorUse
===instead of==to avoid type coercion issues.- if(dispensableAssets.length == 0 && permittedChains.length == 0) { + if(dispensableAssets.length === 0 && permittedChains.length === 0) {
95-104: Extract magic numbers to named constantsThe intent configuration uses several magic numbers that should be extracted to named constants for better maintainability.
+const INTENT_CONFIG = { + DEADLINE_SECONDS: 60, + MAX_GAS: 6000000, + DEFAULT_SLIPPAGE_PERCENTAGE: 5 +}; const intent: UserIntent = { constraints: { - deadline: BigInt(Math.floor(Date.now() / 1000)) + BigInt(60), + deadline: BigInt(Math.floor(Date.now() / 1000)) + BigInt(INTENT_CONFIG.DEADLINE_SECONDS), desiredAssets: [{ asset: props.token.address as Hex, chainId: BigInt(props.token.chainId), value: getDesiredAssetValue(input, props.token.decimals, props.token.usdValue) }], dispensableAssets, - maxGas: BigInt(6000000), + maxGas: BigInt(INTENT_CONFIG.MAX_GAS), permittedChains, - slippagePercentage: 5, + slippagePercentage: INTENT_CONFIG.DEFAULT_SLIPPAGE_PERCENTAGE, },
159-163: Add missing dependencies to useEffectThe useEffect hook that calls
manageIntentis missing several dependencies that the function uses.useEffect(() => { if (debouncedUsdAmount) { manageIntent(debouncedUsdAmount) } - }, [debouncedUsdAmount]); + }, [debouncedUsdAmount, props.token, accountAddress, intentSdk, props.walletPortfolioData]);Note: This will cause
manageIntentto be called more frequently. Consider wrapping it inuseCallbackto optimize.
322-322: Add key prop to amount buttonsReact requires key props for list items.
- <div className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}> + <div key={item} className="flex" style={{backgroundColor: "black", marginLeft: 10, width: 75, height: 30, borderRadius: 10}}>
295-310: Extract SVG icons to separate componentsSVG icons should be extracted to their own components to improve code readability and maintainability.
Create an "images" or "icons" folder and extract the wallet icon SVG to a separate component for better organization.
🧹 Nitpick comments (1)
src/apps/pulse/components/Buy.tsx (1)
38-38: Remove console.log statementConsole.log statements should be removed from production code.
- console.log("token:: ", token);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(2 hunks)src/apps/pulse/components/Buy.tsx(1 hunks)src/apps/pulse/components/IntentTracker.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/apps/pulse/components/IntentTracker.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/apps/pulse/components/Buy.tsx
[error] 322-322: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/apps/pulse/utils/parseSearchData.ts (1)
30-81: Add bounds checking for array access.The functions access
decimals[i]andcontracts[i]without verifying these arrays have the same length asblockchains, which could cause runtime errors.Add bounds checking to prevent array access errors:
for (let i = 0; i < blockchains.length; i += 1) { if ( MOBULA_CHAIN_NAMES.includes(blockchains[i]) && + i < contracts.length && + i < decimals.length && (chains === MobulaChainNames.All || chains === blockchains[i]) ) {src/apps/pulse/components/Search.tsx (3)
51-54: Use props.isBuy instead of hardcoded value.The token search hook uses a hardcoded
{isBuy: true}but should use theisBuyvalue from props.const { searchText, setSearchText, searchData, isFetching } = useTokenSearch({ - isBuy: true, + isBuy, chains, });
59-62: Avoid non-null assertion on potentially undefined data.Using the non-null assertion operator
!onsearchData?.result.data!is risky since the data could still be undefined.let list; if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains); + list = parseSearchData(searchData.result.data, chains); }
263-298: Add key prop to category buttons.React requires key props when rendering lists for efficient DOM updates.
{['🔥 Trending', '🌱 Fresh', '🚀 Top Gainers', '💰My Holdings'].map( (item, index) => { return ( <div + key={item} className="flex"
🧹 Nitpick comments (2)
src/apps/pulse/utils/number.ts (1)
14-36: Add input validation to parseNumberString.The function should handle edge cases better, particularly for malformed inputs with incorrect comma placement.
Consider adding more robust validation:
export function parseNumberString(input: string): number { + if (!input || typeof input !== 'string') return 0; + const match = input.match(/^([\d,.]+)([KMB]?)$/i); if (!match) return 0; const [, num, unit] = match; + // Validate comma placement (basic check) + if (num.includes(',') && !/^\d{1,3}(,\d{3})*(\.\d+)?$/.test(num)) { + return 0; + } let value = parseFloat(num.replace(/,/g, '')); + if (isNaN(value)) return 0;src/apps/pulse/components/ChainOverlay.tsx (1)
54-107: Extract inline SVG to a separate component or icon library.The globe SVG is quite large (53 lines) and makes the component harder to read. Consider extracting it to improve maintainability.
Create a separate
GlobeIconcomponent:const GlobeIcon = () => ( <svg width="24" height="24" viewBox="0 0 20 20" fill="none"> {/* SVG paths */} </svg> );Then use it in the component:
- {/* Inline SVG for globe */} - <svg - width="24" - height="24" - // ... rest of SVG - </svg> + <GlobeIcon />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/apps/pulse/components/ChainOverlay.tsx(1 hunks)src/apps/pulse/components/Search.tsx(1 hunks)src/apps/pulse/components/Sort.tsx(1 hunks)src/apps/pulse/components/TokenList.tsx(1 hunks)src/apps/pulse/types/tokens.ts(1 hunks)src/apps/pulse/utils/number.ts(1 hunks)src/apps/pulse/utils/parseSearchData.ts(1 hunks)src/apps/pulse/utils/time.ts(1 hunks)src/types/api.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/api.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: in this repository, tiletitleprops and tiletitle are different types that serve different purposes. ...
Learnt from: RanaBug
PR: pillarwallet/x#290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Sort.tsxsrc/apps/pulse/components/ChainOverlay.tsxsrc/apps/pulse/types/tokens.tssrc/apps/pulse/components/TokenList.tsxsrc/apps/pulse/components/Search.tsx
📚 Learning: in this repository, tiletitleprops and tiletitle are different types that serve different purposes. ...
Learnt from: RanaBug
PR: pillarwallet/x#290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pulse/components/Sort.tsxsrc/apps/pulse/types/tokens.tssrc/apps/pulse/components/TokenList.tsx
📚 Learning: in the exchange app, `swaptokenlist` and `receivetokenlist` are derived from `searchtokenresult` whe...
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/types/tokens.tssrc/apps/pulse/components/TokenList.tsxsrc/apps/pulse/components/Search.tsxsrc/apps/pulse/utils/parseSearchData.ts
📚 Learning: in src/apps/leaderboard/utils/index.tsx, the getlastweekmigrationdata function intentionally uses cu...
Learnt from: RanaBug
PR: pillarwallet/x#334
File: src/apps/leaderboard/utils/index.tsx:91-94
Timestamp: 2025-06-17T09:20:44.533Z
Learning: In src/apps/leaderboard/utils/index.tsx, the getLastWeekMigrationData function intentionally uses currentWeek (not lastWeek) for the completedSwapWeek lookup. This is correct business logic - when retrieving last week's migration data, the function should check swap completion against the current week while using lastWeek for points and USD calculations.
Applied to files:
src/apps/pulse/components/Search.tsx
🧬 Code Graph Analysis (1)
src/apps/pulse/types/tokens.ts (1)
src/apps/pulse/components/PayingToken.tsx (1)
PayingToken(8-61)
🪛 Biome (2.1.2)
src/apps/pulse/components/TokenList.tsx
[error] 125-136: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Search.tsx
[error] 267-276: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ 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: build
🔇 Additional comments (9)
src/apps/pulse/types/tokens.ts (1)
1-32: Well-structured type definitions.The type definitions are properly exported and well-structured. The
SelectedTokenandPayingTokentypes align perfectly with their usage in components likePayingToken.tsx, and the enums provide clear constants for search and sorting functionality.src/apps/pulse/components/Sort.tsx (1)
7-29: Clean and effective sort indicator component.The component correctly uses conditional opacity to highlight the active sort direction and follows React best practices with proper props destructuring and SVG implementation.
src/apps/pulse/components/TokenList.tsx (2)
1-29: LGTM! Clean component structure and imports.The component imports and interface definition are well-structured with proper TypeScript typing and appropriate dependencies.
58-255: LGTM! Proper conditional rendering pattern.The conditional rendering logic correctly handles the case when no assets are provided, returning an empty fragment with appropriate ESLint disable comment.
src/apps/pulse/utils/parseSearchData.ts (3)
15-28: LGTM! Well-defined Asset type.The Asset type definition is comprehensive with appropriate nullable types and covers all necessary token properties.
83-98: LGTM! Proper type handling and parsing logic.The function correctly uses type guards to handle mixed response types and appropriately delegates to the specific parsing functions.
100-131: LGTM! Robust parsing with proper fallbacks.The function safely handles complex nested data structures with appropriate optional chaining, fallback values, and defensive programming practices.
src/apps/pulse/components/Search.tsx (2)
148-159: Hardcoded sell token data remains - intentional per previous discussion.The sell token is still hardcoded to USDC as noted in previous reviews. Based on earlier discussion, this is intentional until the sell flow is completed.
Consider adding a TODO comment to track this for future implementation:
} else { + // TODO: Implement proper sell token selection when sell flow is completed setSellToken({
165-338: LGTM! Well-structured component with proper React patterns.The component demonstrates good React practices with proper hooks usage, event handling, and state management. The UI structure is clean and follows appropriate patterns for search functionality.
| <div | ||
| style={{ | ||
| position: 'fixed', | ||
| top: 0, | ||
| left: 0, | ||
| width: 200, | ||
| height: 210, | ||
| zIndex: 1999, | ||
| }} | ||
| onClick={() => { | ||
| setShowChainOverlay(false); | ||
| setOverlayStyle({}); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS classes instead of fixed inline dimensions.
The fixed backdrop dimensions (200x210) and extensive inline styling make the component hard to maintain and adapt to different screen sizes.
Consider moving styles to CSS classes and making the backdrop cover the full screen:
- style={{
- position: 'fixed',
- top: 0,
- left: 0,
- width: 200,
- height: 210,
- zIndex: 1999,
- }}
+ className="fixed inset-0 z-[1999] bg-black bg-opacity-20"📝 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.
| <div | |
| style={{ | |
| position: 'fixed', | |
| top: 0, | |
| left: 0, | |
| width: 200, | |
| height: 210, | |
| zIndex: 1999, | |
| }} | |
| onClick={() => { | |
| setShowChainOverlay(false); | |
| setOverlayStyle({}); | |
| }} | |
| /> | |
| <div | |
| className="fixed inset-0 z-[1999] bg-black bg-opacity-20" | |
| onClick={() => { | |
| setShowChainOverlay(false); | |
| setOverlayStyle({}); | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In src/apps/pulse/components/ChainOverlay.tsx around lines 23 to 36, the inline
styles use fixed width and height (200x210) which limits responsiveness and
maintainability. Refactor by moving these styles into a CSS class, update the
class to make the backdrop cover the full screen (e.g., width and height 100% or
100vw/100vh), and replace the inline style with this class. This will improve
adaptability and keep styling consistent.
| const handleSortChange = ( | ||
| key: 'mCap' | 'volume' | 'price' | 'priceChange24h' | ||
| ) => { | ||
| const sortType = | ||
| // eslint-disable-next-line no-nested-ternary | ||
| sort[key] === SortType.Down | ||
| ? SortType.Up | ||
| : sort[key] === SortType.Up | ||
| ? SortType.Down | ||
| : SortType.Up; | ||
|
|
||
| assets.sort((a, b) => { | ||
| if (sortType === SortType.Up) { | ||
| return (b[key] || 0) - (a[key] || 0); | ||
| } | ||
| return (a[key] || 0) - (b[key] || 0); | ||
| }); | ||
|
|
||
| setSort({ | ||
| mCap: undefined, | ||
| price: undefined, | ||
| priceChange24h: undefined, | ||
| volume: undefined, | ||
| [key]: sortType, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Fix prop mutation - create a copy before sorting.
The function directly mutates the assets prop array, which violates React principles and can cause unexpected behavior.
Apply this fix to create a copy before sorting:
const handleSortChange = (
key: 'mCap' | 'volume' | 'price' | 'priceChange24h'
) => {
const sortType =
// eslint-disable-next-line no-nested-ternary
sort[key] === SortType.Down
? SortType.Up
: sort[key] === SortType.Up
? SortType.Down
: SortType.Up;
- assets.sort((a, b) => {
+ const sortedAssets = [...assets].sort((a, b) => {
if (sortType === SortType.Up) {
return (b[key] || 0) - (a[key] || 0);
}
return (a[key] || 0) - (b[key] || 0);
});
+ // You'll need to pass sortedAssets back to parent or manage it in state
setSort({
mCap: undefined,
price: undefined,
priceChange24h: undefined,
volume: undefined,
[key]: sortType,
});
};Consider refactoring to either:
- Pass sorted assets back to parent component via callback
- Manage sorted assets in local component state
📝 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.
| const handleSortChange = ( | |
| key: 'mCap' | 'volume' | 'price' | 'priceChange24h' | |
| ) => { | |
| const sortType = | |
| // eslint-disable-next-line no-nested-ternary | |
| sort[key] === SortType.Down | |
| ? SortType.Up | |
| : sort[key] === SortType.Up | |
| ? SortType.Down | |
| : SortType.Up; | |
| assets.sort((a, b) => { | |
| if (sortType === SortType.Up) { | |
| return (b[key] || 0) - (a[key] || 0); | |
| } | |
| return (a[key] || 0) - (b[key] || 0); | |
| }); | |
| setSort({ | |
| mCap: undefined, | |
| price: undefined, | |
| priceChange24h: undefined, | |
| volume: undefined, | |
| [key]: sortType, | |
| }); | |
| }; | |
| const handleSortChange = ( | |
| key: 'mCap' | 'volume' | 'price' | 'priceChange24h' | |
| ) => { | |
| const sortType = | |
| // eslint-disable-next-line no-nested-ternary | |
| sort[key] === SortType.Down | |
| ? SortType.Up | |
| : sort[key] === SortType.Up | |
| ? SortType.Down | |
| : SortType.Up; | |
| const sortedAssets = [...assets].sort((a, b) => { | |
| if (sortType === SortType.Up) { | |
| return (b[key] || 0) - (a[key] || 0); | |
| } | |
| return (a[key] || 0) - (b[key] || 0); | |
| }); | |
| // You'll need to pass sortedAssets back to parent or manage it in state | |
| setSort({ | |
| mCap: undefined, | |
| price: undefined, | |
| priceChange24h: undefined, | |
| volume: undefined, | |
| [key]: sortType, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/apps/pulse/components/TokenList.tsx lines 31 to 56, the handleSortChange
function directly mutates the assets prop by sorting it in place, which violates
React's immutability principles. To fix this, create a shallow copy of the
assets array before sorting it, for example using slice() or spread syntax, then
sort the copy instead. Additionally, consider managing the sorted assets in
local state or passing the sorted array back to the parent component via a
callback to avoid mutating props directly.
| {assets.map((item) => { | ||
| return ( | ||
| <button | ||
| className="flex w-full" | ||
| style={{ | ||
| height: 36, | ||
| marginTop: 10, | ||
| marginBottom: 10, | ||
| }} | ||
| onClick={() => { | ||
| handleTokenSelect(item); | ||
| }} | ||
| type="button" | ||
| > | ||
| <div style={{ position: 'relative', display: 'inline-block' }}> | ||
| {item.logo ? ( | ||
| <img | ||
| src={item.logo || ''} | ||
| style={{ | ||
| width: 36, | ||
| height: 36, | ||
| marginLeft: 10, | ||
| borderRadius: 50, | ||
| }} | ||
| alt="token logo" | ||
| /> | ||
| ) : ( | ||
| <div | ||
| className="w-full h-full overflow-hidden rounded-full" | ||
| style={{ | ||
| width: 36, | ||
| height: 36, | ||
| borderRadius: 50, | ||
| marginLeft: 10, | ||
| }} | ||
| > | ||
| <RandomAvatar name={item.name || ''} /> | ||
| <span | ||
| className="absolute inset-0 flex items-center justify-center text-white text-lg font-bold" | ||
| style={{ marginLeft: 10 }} | ||
| > | ||
| {item.name?.slice(0, 2)} | ||
| </span> | ||
| </div> | ||
| )} | ||
| <img | ||
| src={getLogoForChainId( | ||
| chainNameToChainIdTokensData(item.chain) | ||
| )} | ||
| style={{ | ||
| position: 'absolute', | ||
| bottom: '-2px', | ||
| right: '-2px', | ||
| width: 15, | ||
| height: 15, | ||
| borderRadius: '50%', | ||
| }} | ||
| alt="chain logo" | ||
| /> | ||
| </div> | ||
| <div | ||
| className="flex flex-col" | ||
| style={{ width: 250, height: 14, marginLeft: 10 }} | ||
| > | ||
| <div className="flex"> | ||
| <p style={{ fontSize: 13, fontWeight: 400 }}>{item.symbol}</p> | ||
| <p | ||
| style={{ | ||
| fontSize: 12, | ||
| fontWeight: 400, | ||
| marginLeft: 3, | ||
| color: 'grey', | ||
| }} | ||
| > | ||
| {item.name} | ||
| </p> | ||
| </div> | ||
| <div className="flex"> | ||
| {searchType === SearchType.Fresh && ( | ||
| <p | ||
| style={{ fontSize: 13, fontWeight: 400, marginRight: 3 }} | ||
| > | ||
| {formatElapsedTime(item.timestamp)} | ||
| </p> | ||
| )} | ||
| <p style={{ fontSize: 13, fontWeight: 400, color: 'grey' }}> | ||
| MCap: | ||
| </p> | ||
| <p | ||
| style={{ | ||
| fontSize: 13, | ||
| fontWeight: 400, | ||
| marginLeft: 3, | ||
| }} | ||
| > | ||
| {formatBigNumber(item.mCap || 0)} | ||
| </p> | ||
| <p | ||
| style={{ | ||
| fontSize: 13, | ||
| fontWeight: 400, | ||
| marginLeft: 3, | ||
| color: 'grey', | ||
| }} | ||
| > | ||
| Vol: | ||
| </p> | ||
| <p | ||
| style={{ | ||
| fontSize: 13, | ||
| fontWeight: 400, | ||
| marginLeft: 3, | ||
| }} | ||
| > | ||
| {formatBigNumber(item.volume || 0)} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <div className="flex flex-col ml-auto mr-2.5"> | ||
| <div>{formatTokenPriceDisplay(item.price ?? 0)}</div> | ||
| <div className="ml-auto"> | ||
| {formatPriceChangeDisplay(item.priceChange24h ?? 0)} | ||
| </div> | ||
| </div> | ||
| </button> | ||
| ); |
There was a problem hiding this comment.
Add missing key prop to mapped token buttons.
The mapped token buttons are missing the required key prop, which can cause React reconciliation issues.
Apply this fix:
{assets.map((item) => {
return (
<button
+ key={`${item.contract}-${item.chain}`}
className="flex w-full"Using a combination of contract address and chain ensures uniqueness across different networks.
📝 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.
| {assets.map((item) => { | |
| return ( | |
| <button | |
| className="flex w-full" | |
| style={{ | |
| height: 36, | |
| marginTop: 10, | |
| marginBottom: 10, | |
| }} | |
| onClick={() => { | |
| handleTokenSelect(item); | |
| }} | |
| type="button" | |
| > | |
| <div style={{ position: 'relative', display: 'inline-block' }}> | |
| {item.logo ? ( | |
| <img | |
| src={item.logo || ''} | |
| style={{ | |
| width: 36, | |
| height: 36, | |
| marginLeft: 10, | |
| borderRadius: 50, | |
| }} | |
| alt="token logo" | |
| /> | |
| ) : ( | |
| <div | |
| className="w-full h-full overflow-hidden rounded-full" | |
| style={{ | |
| width: 36, | |
| height: 36, | |
| borderRadius: 50, | |
| marginLeft: 10, | |
| }} | |
| > | |
| <RandomAvatar name={item.name || ''} /> | |
| <span | |
| className="absolute inset-0 flex items-center justify-center text-white text-lg font-bold" | |
| style={{ marginLeft: 10 }} | |
| > | |
| {item.name?.slice(0, 2)} | |
| </span> | |
| </div> | |
| )} | |
| <img | |
| src={getLogoForChainId( | |
| chainNameToChainIdTokensData(item.chain) | |
| )} | |
| style={{ | |
| position: 'absolute', | |
| bottom: '-2px', | |
| right: '-2px', | |
| width: 15, | |
| height: 15, | |
| borderRadius: '50%', | |
| }} | |
| alt="chain logo" | |
| /> | |
| </div> | |
| <div | |
| className="flex flex-col" | |
| style={{ width: 250, height: 14, marginLeft: 10 }} | |
| > | |
| <div className="flex"> | |
| <p style={{ fontSize: 13, fontWeight: 400 }}>{item.symbol}</p> | |
| <p | |
| style={{ | |
| fontSize: 12, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| color: 'grey', | |
| }} | |
| > | |
| {item.name} | |
| </p> | |
| </div> | |
| <div className="flex"> | |
| {searchType === SearchType.Fresh && ( | |
| <p | |
| style={{ fontSize: 13, fontWeight: 400, marginRight: 3 }} | |
| > | |
| {formatElapsedTime(item.timestamp)} | |
| </p> | |
| )} | |
| <p style={{ fontSize: 13, fontWeight: 400, color: 'grey' }}> | |
| MCap: | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| }} | |
| > | |
| {formatBigNumber(item.mCap || 0)} | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| color: 'grey', | |
| }} | |
| > | |
| Vol: | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| }} | |
| > | |
| {formatBigNumber(item.volume || 0)} | |
| </p> | |
| </div> | |
| </div> | |
| <div className="flex flex-col ml-auto mr-2.5"> | |
| <div>{formatTokenPriceDisplay(item.price ?? 0)}</div> | |
| <div className="ml-auto"> | |
| {formatPriceChangeDisplay(item.priceChange24h ?? 0)} | |
| </div> | |
| </div> | |
| </button> | |
| ); | |
| {assets.map((item) => { | |
| return ( | |
| <button | |
| key={`${item.contract}-${item.chain}`} | |
| className="flex w-full" | |
| style={{ | |
| height: 36, | |
| marginTop: 10, | |
| marginBottom: 10, | |
| }} | |
| onClick={() => { | |
| handleTokenSelect(item); | |
| }} | |
| type="button" | |
| > | |
| <div style={{ position: 'relative', display: 'inline-block' }}> | |
| {item.logo ? ( | |
| <img | |
| src={item.logo || ''} | |
| style={{ | |
| width: 36, | |
| height: 36, | |
| marginLeft: 10, | |
| borderRadius: 50, | |
| }} | |
| alt="token logo" | |
| /> | |
| ) : ( | |
| <div | |
| className="w-full h-full overflow-hidden rounded-full" | |
| style={{ | |
| width: 36, | |
| height: 36, | |
| borderRadius: 50, | |
| marginLeft: 10, | |
| }} | |
| > | |
| <RandomAvatar name={item.name || ''} /> | |
| <span | |
| className="absolute inset-0 flex items-center justify-center text-white text-lg font-bold" | |
| style={{ marginLeft: 10 }} | |
| > | |
| {item.name?.slice(0, 2)} | |
| </span> | |
| </div> | |
| )} | |
| <img | |
| src={getLogoForChainId( | |
| chainNameToChainIdTokensData(item.chain) | |
| )} | |
| style={{ | |
| position: 'absolute', | |
| bottom: '-2px', | |
| right: '-2px', | |
| width: 15, | |
| height: 15, | |
| borderRadius: '50%', | |
| }} | |
| alt="chain logo" | |
| /> | |
| </div> | |
| <div | |
| className="flex flex-col" | |
| style={{ width: 250, height: 14, marginLeft: 10 }} | |
| > | |
| <div className="flex"> | |
| <p style={{ fontSize: 13, fontWeight: 400 }}>{item.symbol}</p> | |
| <p | |
| style={{ | |
| fontSize: 12, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| color: 'grey', | |
| }} | |
| > | |
| {item.name} | |
| </p> | |
| </div> | |
| <div className="flex"> | |
| {searchType === SearchType.Fresh && ( | |
| <p | |
| style={{ fontSize: 13, fontWeight: 400, marginRight: 3 }} | |
| > | |
| {formatElapsedTime(item.timestamp)} | |
| </p> | |
| )} | |
| <p style={{ fontSize: 13, fontWeight: 400, color: 'grey' }}> | |
| MCap: | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| }} | |
| > | |
| {formatBigNumber(item.mCap || 0)} | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| color: 'grey', | |
| }} | |
| > | |
| Vol: | |
| </p> | |
| <p | |
| style={{ | |
| fontSize: 13, | |
| fontWeight: 400, | |
| marginLeft: 3, | |
| }} | |
| > | |
| {formatBigNumber(item.volume || 0)} | |
| </p> | |
| </div> | |
| </div> | |
| <div className="flex flex-col ml-auto mr-2.5"> | |
| <div>{formatTokenPriceDisplay(item.price ?? 0)}</div> | |
| <div className="ml-auto"> | |
| {formatPriceChangeDisplay(item.priceChange24h ?? 0)} | |
| </div> | |
| </div> | |
| </button> | |
| ); | |
| })} |
🧰 Tools
🪛 Biome (2.1.2)
[error] 125-136: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In src/apps/pulse/components/TokenList.tsx between lines 123 and 248, the mapped
token buttons inside the assets.map call lack a unique key prop, which is
necessary for React to efficiently track list items. Fix this by adding a key
prop to the button element, using a unique identifier such as a combination of
the token's contract address and chain to ensure uniqueness across different
networks.
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (12)
src/apps/pulse/components/Misc/Close.tsx (1)
6-13: A11y + minor cleanup: avoid redundant wrapper and mark icon decorative
- onClick wrapper is unnecessary.
- Image should be decorative since the button already has an aria-label; prevent double announcement.
return ( <button style={{ height: 20, width: 20, marginRight: 10 }} - onClick={() => onClose()} + onClick={onClose} type="button" aria-label="Close" > - <img src={CloseIcon} alt="close-icon" /> + <img src={CloseIcon} alt="" aria-hidden="true" width={20} height={20} /> </button> );src/apps/pulse/components/Misc/Refresh.tsx (1)
3-13: Make the button usable: accept onClick prop and fix icon a11yExpose an onClick for consumers; mark the icon decorative to avoid duplicate announcements.
-export default function Refresh() { +export default function Refresh({ onClick }: { onClick?: () => void }) { return ( <button - className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]" + className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]" + onClick={onClick} type="button" aria-label="Refresh" > - <img src={RefreshIcon} alt="refresh-icon" /> + <img src={RefreshIcon} alt="" aria-hidden="true" /> </button> ); }src/apps/pulse/components/App/AppWrapper.tsx (2)
17-23: Avoid defining a custom hook-like helper inside the componentDefining useQuery inside the component and returning a new URLSearchParams each render creates an unstable dependency downstream. Prefer using useLocation directly.
- const useQuery = () => { - const { search } = useLocation(); - return new URLSearchParams(search); - }; - - const query = useQuery(); + const { search } = useLocation();
14-16: Remove lint disable by intentionally discarding the first tuple valueNo need for an eslint-disable; discard the unused state value explicitly.
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [sellToken, setSellToken] = useState<SelectedToken | null>(null); + const [, setSellToken] = useState<SelectedToken | null>(null);src/apps/pulse/components/Search/Sort.tsx (1)
10-27: Add accessibility attributes to non-interactive SVGThe icon is purely decorative, so add
aria-hidden="true" focusable="false"on the<svg>element (or wrap it in a<span role="img" aria-label="sort">) to prevent it from being announced by screen readers.src/apps/pulse/components/Buy/PayingToken.tsx (2)
17-27: Prefer utility classes over large inline-style blocksInline styles hinder theming and dark-mode support. Consider replacing width/height/borderRadius/margins with tailwind utilities (
w-8 h-8 rounded-full ml-1 mr-1, etc.) for consistency with the rest of the codebase.
48-57: Format numeric amounts for locale & precision
{payingToken.totalRaw}and${payingToken.totalUsd.toFixed(2)}assume EN locale and may show long digit strings. Use a sharedformatNumberhelper (e.g.Intl.NumberFormat) to add thousands separators and consistent decimals.src/apps/pulse/components/Price/TokenPriceChange.tsx (1)
48-53: Handle zero change as neutral
value > 0 ? green : redpaints a 0 % change red. Consider a neutral colour & no arrow whenvalue === 0to avoid misleading users.src/apps/pulse/components/Status/TxStatus.tsx (1)
37-44: Gracefully handle unknown chain IDs
getBlockScanreturns an empty string for unrecognised chains, yielding a broken link (href="/<hash>"). Guard withif (!txUrl) return nullor show plain hash text instead.src/apps/pulse/components/App/HomeScreen.tsx (1)
65-71: Placeholder search hint is non-interactiveConsider turning the whole search bar into an
<input>or at least give itrole="button"so screen-reader users know it’s clickable.src/apps/pulse/components/Search/Search.tsx (1)
103-118: Cancel the in-flight fetch on unmount / param change
fetch()is fired every timesearchTypeorchainschange, but the promise isn’t aborted.
Add anAbortControllerto avoid setting state on an unmounted component.src/apps/pulse/components/Buy/BuyButton.tsx (1)
90-99: Avoid double invocation ofisDisabled()
isDisabled()is executed twice per render (fordisabledandstyle). Cache its value in aconstto avoid redundant work and ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pulse/assets/arrow-down.svgis excluded by!**/*.svgsrc/apps/pulse/assets/close-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/copy-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/esc-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/globe-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/moreinfo-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/new-tab.svgis excluded by!**/*.svgsrc/apps/pulse/assets/refresh-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/seach-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/selected-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/setting-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/wallet.svgis excluded by!**/*.svgsrc/apps/pulse/assets/warning.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
src/apps/pulse/components/App/AppWrapper.tsx(1 hunks)src/apps/pulse/components/App/HomeScreen.tsx(1 hunks)src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Buy/BuyButton.tsx(1 hunks)src/apps/pulse/components/Buy/PayingToken.tsx(1 hunks)src/apps/pulse/components/Buy/PreviewBuy.tsx(1 hunks)src/apps/pulse/components/Misc/Close.tsx(1 hunks)src/apps/pulse/components/Misc/Esc.tsx(1 hunks)src/apps/pulse/components/Misc/Refresh.tsx(1 hunks)src/apps/pulse/components/Misc/Settings.tsx(1 hunks)src/apps/pulse/components/Price/TokenPrice.tsx(1 hunks)src/apps/pulse/components/Price/TokenPriceChange.tsx(1 hunks)src/apps/pulse/components/Search/ChainOverlay.tsx(1 hunks)src/apps/pulse/components/Search/ChainSelect.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(1 hunks)src/apps/pulse/components/Search/Sort.tsx(1 hunks)src/apps/pulse/components/Search/TokenList.tsx(1 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/apps/pulse/components/Status/IntentTracker.tsx(1 hunks)src/apps/pulse/components/Status/TxStatus.tsx(1 hunks)src/apps/pulse/index.tsx(1 hunks)src/apps/pulse/utils/intent.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/apps/pulse/components/Search/ChainSelect.tsx
- src/apps/pulse/components/Misc/Settings.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/index.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/App/AppWrapper.tsxsrc/apps/pulse/components/Buy/PayingToken.tsxsrc/apps/pulse/components/Status/IntentTracker.tsxsrc/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
PR: pillarwallet/x#290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Search/Sort.tsxsrc/apps/pulse/components/Search/ChainOverlay.tsxsrc/apps/pulse/components/Price/TokenPriceChange.tsxsrc/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/components/Buy/PayingToken.tsxsrc/apps/pulse/components/Status/IntentTracker.tsxsrc/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Status/TxStatus.tsxsrc/apps/pulse/components/Buy/BuyButton.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
PR: pillarwallet/x#290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pulse/components/Search/Sort.tsxsrc/apps/pulse/components/Price/TokenPriceChange.tsxsrc/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/components/Buy/PayingToken.tsxsrc/apps/pulse/components/Buy/PreviewBuy.tsxsrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-05-28T14:30:02.702Z
Learnt from: RanaBug
PR: pillarwallet/x#320
File: src/components/BottomMenuModal/HistoryModal/TransactionInfo.tsx:177-182
Timestamp: 2025-05-28T14:30:02.702Z
Learning: In the transaction history tracking feature, chain ID validation is performed earlier in the flow before the values reach the TransactionInfo component, so additional validation at the display level is not needed.
Applied to files:
src/apps/pulse/components/Status/TxStatus.tsx
🧬 Code Graph Analysis (6)
src/apps/pulse/components/App/AppWrapper.tsx (2)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/components/Search/Search.tsx (1)
Search(49-327)
src/apps/pulse/components/Buy/PayingToken.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(158-192)
src/apps/pulse/components/Status/IntentTracker.tsx (3)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/hooks/useIntentSdk.ts (1)
useIntentSdk(7-39)src/apps/pulse/components/Misc/Esc.tsx (1)
Esc(7-26)
src/apps/pulse/components/Status/TxStatus.tsx (1)
src/utils/blockchain.ts (1)
getBlockScan(218-237)
src/apps/pulse/components/Search/TokenList.tsx (7)
src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/time.ts (1)
formatElapsedTime(1-25)src/apps/pulse/utils/number.ts (1)
formatBigNumber(1-12)src/apps/pulse/components/Price/TokenPrice.tsx (1)
TokenPrice(5-33)src/apps/pulse/components/Price/TokenPriceChange.tsx (1)
TokenPriceChange(5-56)
src/apps/pulse/utils/intent.ts (3)
src/types/api.ts (1)
PortfolioData(738-747)src/apps/pulse/components/Buy/PayingToken.tsx (1)
PayingToken(8-61)src/apps/pulse/constants/tokens.ts (1)
STABLE_CURRENCIES(1-8)
🪛 Biome (2.1.2)
src/apps/pulse/components/Sell/Sell.tsx
[error] 19-28: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Buy/PreviewBuy.tsx
[error] 134-135: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Search/Search.tsx
[error] 256-265: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Search/TokenList.tsx
[error] 123-134: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/apps/pulse/components/Buy/Buy.tsx
[error] 386-396: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ 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: build
🔇 Additional comments (2)
src/apps/pulse/components/Status/IntentTracker.tsx (1)
59-88: Polling starts even when SDK isn’t ready → busy loop
poll()is scheduled regardless ofintentSdkbeingnull; the early return skips the body but still re-queues another timeout, wasting cycles.Guard the effect:
-useEffect(() => { +useEffect(() => { + if (!intentSdk) return;and clear the timer with
clearTimeout(id)in the cleanup for completeness.Likely an incorrect or invalid review comment.
src/apps/pulse/components/Search/Search.tsx (1)
135-160: Hard-coded sell token looks accidentalWhen
isBuyisfalsethe selected token is replaced with a fixed USDC object, discarding the user’s choice and chain.
Confirm this is intentional; otherwise derive the token fromitemjust like the buy-path.
| export function getDesiredAssetValue( | ||
| input: string, | ||
| decimals: number, | ||
| usdValue: string | ||
| ): bigint { | ||
| const value = Number(parseFloat(input)) / Number(parseFloat(usdValue)); | ||
| return BigInt(Math.ceil(Number(value.toFixed(6))) * 10 ** decimals); | ||
| } |
There was a problem hiding this comment.
Incorrect / unsafe bigint conversion
value.toFixed(6)+Math.ceilcollapses everything below1to1, inflating the amount.10 ** decimalsoverflows JSNumberfor 18-dec tokens, so precision is already lost beforeBigInt().
-export function getDesiredAssetValue(input: string, decimals: number, usdValue: string): bigint {
- const value = Number(parseFloat(input)) / Number(parseFloat(usdValue));
- return BigInt(Math.ceil(Number(value.toFixed(6))) * 10 ** decimals);
+export function getDesiredAssetValue(input: string, decimals: number, usdValue: string): bigint {
+ const usd = parseFloat(input);
+ const price = parseFloat(usdValue);
+ if (Number.isNaN(usd) || Number.isNaN(price) || price <= 0) return 0n;
+
+ const multiplier = 10n ** BigInt(decimals);
+ const tokens = BigInt(Math.ceil((usd / price) * Number(multiplier)));
+ return tokens;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/apps/pulse/utils/intent.ts lines 8 to 15, the current conversion to
bigint is unsafe because using value.toFixed(6) with Math.ceil inflates values
below 1 to 1, and 10 ** decimals overflows Number for large decimals like 18,
causing precision loss. To fix this, avoid using toFixed and Math.ceil on
floating values; instead, perform all calculations using BigInt or a precise
decimal library to handle large exponentiation safely without overflow, ensuring
accurate scaling by decimals before converting to BigInt.
There was a problem hiding this comment.
@IAmKio It looks like your comment got cut off mid-sentence. You mentioned "This is valid due to" but didn't complete the explanation.
Could you please finish your reasoning for why you believe the current bigint conversion approach is valid? I'd like to understand your perspective on the potential overflow and precision issues I identified with the 10 ** decimals calculation and the toFixed(6) + Math.ceil combination.
| asset: tokenItem.address as Hex, | ||
| chainId: BigInt(tokenItem.chainId), | ||
| maxValue: BigInt( | ||
| Math.ceil( | ||
| Number( | ||
| (Number(Number(input).toFixed(6)) / price).toPrecision(6) | ||
| ) * | ||
| 10 ** token.decimals | ||
| ) | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same precision issue when computing maxValue
10 ** token.decimals is unsafe; use 10n ** BigInt(token.decimals) and perform the calculation in bigint math to keep accuracy.
🤖 Prompt for AI Agents
In src/apps/pulse/utils/intent.ts around lines 44 to 53, the calculation of
maxValue uses floating-point math and the expression 10 ** token.decimals, which
is unsafe for large decimals. To fix this, convert token.decimals to a BigInt
and replace 10 ** token.decimals with 10n ** BigInt(token.decimals), then
perform the entire maxValue calculation using bigint arithmetic to maintain
precision and avoid floating-point errors.
There was a problem hiding this comment.
This is valid due to decimal place limitations in JavaScript
There was a problem hiding this comment.
@IAmKio, I understand your perspective. You've made a conscious decision to accept the floating-point precision limitations as a trade-off in this implementation, likely considering the practical impact for your use case.
✏️ Learnings added
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.646Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
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.
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
LGTM - please address the CodeRabbit issues then it can be merged
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (6)
src/apps/pulse/components/Price/TokenPrice.tsx (1)
13-15: Make zero rendering consistent with 5-decimal outputElsewhere you show 5 decimals; zero currently renders with 2. Use 5 for consistency.
- if (firstNonZeroIndex < 0) { - return <p style={{ fontSize: 13, fontWeight: 400 }}>$0.00</p>; - } + if (firstNonZeroIndex < 0) { + return <p style={{ fontSize: 13, fontWeight: 400 }}>$0.00000</p>; + }src/apps/pulse/utils/intent.ts (2)
17-19: LGTM! Using the team's floating-point approach with bigIntPow.The conversion now uses the
bigIntPowutility for safe bigint exponentiation, addressing the overflow concerns while maintaining the team's accepted trade-off with floating-point precision.
51-62: LGTM! Consistent application of bigIntPow for safe calculations.The maxValue calculation now correctly uses
bigIntPowfor both the token decimals and the 10^6 divisor, maintaining consistency with the team's approach to floating-point limitations.src/apps/pulse/hooks/useModularSdk.ts (1)
44-64: Missing cleanup for async operations.The useEffect that initializes the SDK doesn't prevent state updates on unmounted components. This could lead to memory leaks or React warnings.
Add cleanup to prevent memory leaks:
useEffect(() => { + let cancelled = false; + if (ready && authenticated && walletProvider && payingTokens.length > 0) { const { chainId } = payingTokens[0]; const options: SdkOptions = { chainId: payingTokens[0].chainId, bundlerProvider: new EtherspotBundler( chainId, process.env.REACT_APP_ETHERSPOT_BUNDLER_API_KEY || '' ), }; walletProvider.getEthereumProvider().then((provider) => { + if (cancelled) return; + const walletClient = createWalletClient({ account: walletProvider.address as Hex, transport: custom(provider), }); const sdk = new ModularSdk(walletClient, options); setModularSdk(sdk); }); } + + return () => { + cancelled = true; + }; }, [ready, authenticated, walletProvider, payingTokens]);src/apps/pulse/components/Buy/Buy.tsx (1)
384-422: Missing key prop fixed correctly.The component now properly includes a
keyprop for each mapped element, which is essential for React's reconciliation process.src/apps/pulse/components/Misc/Esc.tsx (1)
17-21: Fix applied: aria-label, simplified onClick, and decorative icon look goodThe accessibility fixes from the earlier review are now correctly implemented. Thanks for addressing them.
🧹 Nitpick comments (17)
src/apps/pulse/components/Price/TokenPrice.tsx (4)
17-19: Drop redundant condition:firstNonZeroIndex < 2is never true whenvalue < 0.01For
value < 0.01, the first two decimal digits are always0, sofirstNonZeroIndex >= 2. The second clause never changes the branch outcome.- if (value >= 0.01 || firstNonZeroIndex < 2) { + if (value >= 0.01) { return <p style={{ fontSize: 13, fontWeight: 400 }}>${value.toFixed(5)}</p>; }
21-25: Ensure 4 significant digits even near precision limitWhen
firstNonZeroIndexis close to the end of the 10-decimal window (e.g., 8 or 9),slice(firstNonZeroIndex, firstNonZeroIndex + 4)can return fewer than 4 digits. Pad to always display 4.- const significantDigits = decimals.slice( - firstNonZeroIndex, - firstNonZeroIndex + 4 - ); + const significantDigits = (decimals.slice(firstNonZeroIndex) + '0000').slice(0, 4);
14-15: Replace repeated inline styles with Tailwind classesTailwind is available in this app; switching improves consistency and avoids repeating inline style objects.
- return <p style={{ fontSize: 13, fontWeight: 400 }}>$0.00000</p>; + return <p className="text-[13px] font-normal">$0.00000</p>;- return <p style={{ fontSize: 13, fontWeight: 400 }}>${value.toFixed(5)}</p>; + return <p className="text-[13px] font-normal">${value.toFixed(5)}</p>;- return ( - <p style={{ fontSize: 13, fontWeight: 400 }}> + return ( + <p className="text-[13px] font-normal"> $0.0<sub>{leadingZerosCount}</sub> {significantDigits} </p> );Also applies to: 18-19, 28-31
5-33: Add unit tests for boundary and error-prone casesCover typical and edge values to lock in behavior and prevent regressions (the team accepts float arithmetic; this just validates formatting).
Suggested cases:
- value is NaN → renders "-"
- value is Infinity → renders "-"
- 0 → "$0.00000"
- 9e-11 → "$0.00000"
- 0.0000098765 → "$0.0" + sub(5) + "9876"
- 0.009 → "$0.00900"
- 0.01 → "$0.01000"
- 1.23456789 → "$1.23457"
I can generate RTL/Jest tests if you want me to push a spec file.
src/apps/pulse/utils/intent.ts (2)
26-29: Add TODO tracking for multiple token support.The TODO comment indicates a planned enhancement to support multiple tokens from different chains. This is an important scalability feature for the Pulse app.
Would you like me to create an issue to track the implementation of multi-token, multi-chain dispensable asset logic?
40-44: Consider case-insensitive comparison performance.The current implementation performs
toLowerCase()on every iteration. For better performance with large portfolios, consider normalizing addresses once or using a more efficient lookup mechanism.Consider creating a normalized lookup map:
export function getDispensableAssets( input: string, portfolioData: PortfolioData | undefined ): [DispensableAsset[], bigint[], PayingToken[]] { if (!portfolioData?.assets) { return [[], [], []]; } + // Create normalized lookup map for better performance + const stableCurrencyMap = new Map( + STABLE_CURRENCIES.map(currency => [ + `${currency.chainId}-${currency.address.toLowerCase()}`, + currency + ]) + ); for (const item of portfolioData.assets) { const { price } = item; for (const token of item.contracts_balances) { const tokenItem = { chainId: Number(token.chainId.split(':').at(-1)), address: token.address, }; const usdEq = price * token.balance; - const t = STABLE_CURRENCIES.find( - (x) => - x.address.toLowerCase() === tokenItem.address.toLowerCase() && - x.chainId === tokenItem.chainId - ); + const lookupKey = `${tokenItem.chainId}-${tokenItem.address.toLowerCase()}`; + const t = stableCurrencyMap.get(lookupKey);src/apps/pulse/hooks/useModularSdk.ts (1)
32-42: Consider adding timeout logging for debugging.The
waitForReceiptfunction has a 5-minute timeout but doesn't log when timeout occurs, which could make debugging difficult in production.Add timeout logging for better debugging:
const waitForReceipt = async (userOpHash: string) => { let userOpsReceipt = null; const timeout = Date.now() + 300000; // wait for 5 mins while (userOpsReceipt == null && Date.now() < timeout) { // eslint-disable-next-line no-await-in-loop userOpsReceipt = await modularSdk?.getUserOpReceipt(userOpHash); // eslint-disable-next-line no-await-in-loop await sleep(2); } + if (userOpsReceipt == null) { + console.warn(`Receipt timeout for userOpHash: ${userOpHash}`); + } return userOpsReceipt; };src/apps/pulse/components/Buy/Buy.tsx (4)
95-107: Redundant state updates in dispensable assets logic.The logic unnecessarily sets
dispensableAssetsandpermittedChainsin both branches of the conditional, leading to code duplication.Simplify the conditional logic:
if ( payingTokens.length > 0 && pTokens[0].chainId === payingTokens[0].chainId && pTokens[0].name === payingTokens[0].name && pTokens[0].symbol === payingTokens[0].symbol ) { - setDispensableAssets(dAssets); - setPermittedChains(pChains); - } else { - setDispensableAssets(dAssets); - setPermittedChains(pChains); + // No need to update paying tokens as they're the same + } else { setPayingTokens(pTokens); } + setDispensableAssets(dAssets); + setPermittedChains(pChains);
112-118: Optimize useEffect dependencies.The dependency
dispensableAssets.lengthcould cause unnecessary re-renders. SincedispensableAssetsis derived fromwalletPortfolioDataandusdAmount, it's redundant as a dependency.Remove redundant dependency:
}, [ usdAmount, setPayingTokens, walletPortfolioData?.result.data, - dispensableAssets.length, payingTokens, ]);
61-67: Improve input validation logic.The current validation allows empty strings to pass through, which could cause issues downstream. Consider more explicit validation.
Improve input validation:
const handleUsdAmountChange = (e: React.ChangeEvent<HTMLInputElement>) => { const input = e.target.value; - if (!input || !Number.isNaN(parseFloat(input))) { + if (input === '' || (!Number.isNaN(parseFloat(input)) && parseFloat(input) >= 0)) { setInputPlaceholder('0.00'); setUsdAmount(input); } };
152-153: Verify intentHash Placeholder UsageIt looks like
intentHashis currently hard-coded to the zero hash insrc/apps/pulse/components/Buy/Buy.tsx(around line 152). We didn’t find any internal type or runtime checks in the repo that validate this value, but the SDK may enforce a non-zero or formatted hash at runtime.Please verify with the SDK documentation or implementation whether:
- A zero-hash placeholder is acceptable,
- There’s a provided constant (e.g.
ZERO_INTENT_HASH) you should import instead of inlining, or- You need to generate a real intent hash before sending.
If the zero-hash is only meant as a placeholder, consider replacing the literal with the SDK’s official constant or utility method so it’s clear and less error-prone.
src/apps/pulse/components/Misc/Esc.tsx (3)
21-21: Add explicit icon dimensions to prevent layout shiftDeclare width/height for the icon to avoid CLS and ensure consistent sizing.
- <img src={EscIcon} alt="" aria-hidden="true" /> + <img src={EscIcon} alt="" aria-hidden="true" width={20} height={20} />
10-20: Add focus-visible styles for keyboard accessibilityProvide a visible focus indicator on the button for non-mouse users.
<button + className="focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 focus-visible:ring-offset-black" style={{ backgroundColor: 'black', borderRadius: 10, width: 40, height: 40, }}
7-9: Nit: Inline-destructure props for brevityMinor readability win; no behavior change.
-export default function Esc(props: CloseProps) { - const { closePreview } = props; +export default function Esc({ closePreview }: CloseProps) {src/apps/pulse/components/Status/IntentTracker.tsx (3)
1-1: Remove eslint-disable once anys are goneAfter typing bid/resourceLockInfo, this directive is unnecessary and should be removed to re-enable lint guardrails.
-/* eslint-disable @typescript-eslint/no-explicit-any */
246-257: Center the “Close” label by making the button a flex containerThe current classes won’t center without display: flex.
- <button - className="flex-1 items-center justify-center" + <button + className="flex flex-1 items-center justify-center" style={{ margin: 2, borderRadius: 10,
66-99: Clear scheduled timeouts on unmountDefensive cleanup avoids stray timers after unmount. Your isCancelled flag is good; this complements it.
useEffect(() => { - let isCancelled = false; + let isCancelled = false; + let timeoutId: ReturnType<typeof setTimeout> | null = null; @@ - if (!isCancelled) { - setTimeout(poll, 15000); - } + if (!isCancelled) { + timeoutId = setTimeout(poll, 15000); + } @@ - return () => { - isCancelled = true; // Cleanup - }; + return () => { + isCancelled = true; // Cleanup + if (timeoutId) clearTimeout(timeoutId); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
package.json(2 hunks)src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Buy/PreviewBuy.tsx(1 hunks)src/apps/pulse/components/Misc/Esc.tsx(1 hunks)src/apps/pulse/components/Price/TokenPrice.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(1 hunks)src/apps/pulse/components/Search/TokenList.tsx(1 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/apps/pulse/components/Status/IntentTracker.tsx(1 hunks)src/apps/pulse/constants/tokens.ts(1 hunks)src/apps/pulse/hooks/useModularSdk.ts(1 hunks)src/apps/pulse/utils/intent.ts(1 hunks)src/apps/pulse/utils/number.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/apps/pulse/utils/number.ts
- src/apps/pulse/components/Sell/Sell.tsx
- package.json
- src/apps/pulse/constants/tokens.ts
- src/apps/pulse/components/Search/TokenList.tsx
- src/apps/pulse/components/Search/Search.tsx
- src/apps/pulse/components/Buy/PreviewBuy.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T07:42:24.646Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.646Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/utils/intent.ts
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
PR: pillarwallet/x#290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Price/TokenPrice.tsx
🧬 Code Graph Analysis (3)
src/apps/pulse/components/Buy/Buy.tsx (8)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/types/api.ts (1)
WalletPortfolioMobulaResponse(749-751)src/apps/pulse/components/Buy/PayingToken.tsx (1)
PayingToken(8-61)src/apps/pulse/hooks/useIntentSdk.ts (1)
useIntentSdk(7-39)src/apps/pulse/hooks/useModularSdk.ts (1)
useModularSdk(17-129)src/apps/pulse/utils/intent.ts (2)
getDispensableAssets(22-82)getDesiredAssetValue(9-20)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)src/apps/pulse/components/Buy/BuyButton.tsx (1)
BuyButton(57-112)
src/apps/pulse/hooks/useModularSdk.ts (1)
src/apps/pulse/utils/sleep.ts (1)
sleep(1-5)
src/apps/pulse/utils/intent.ts (3)
src/apps/pulse/utils/number.ts (1)
bigIntPow(41-53)src/types/api.ts (1)
PortfolioData(738-747)src/apps/pulse/constants/tokens.ts (1)
STABLE_CURRENCIES(1-9)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pulse
🔇 Additional comments (1)
src/apps/pulse/hooks/useModularSdk.ts (1)
115-119: Improved error handling for installation process.Good improvement! The error handling now properly resets the installation state in the outer catch block, preventing the UI from getting stuck in a loading state.
| const manageIntent = async (input: string) => { | ||
| setNoEnoughLiquidity(false); | ||
| if ( | ||
| Number(parseFloat(input) > 0) && |
There was a problem hiding this comment.
Fix logical error in amount validation.
Line 124 has Number(parseFloat(input) > 0) which is incorrect - the comparison should be outside the Number() call.
Fix the logical error:
- Number(parseFloat(input) > 0) &&
+ parseFloat(input) > 0 &&📝 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.
| Number(parseFloat(input) > 0) && | |
| parseFloat(input) > 0 && |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy/Buy.tsx around line 124, the expression
currently reads Number(parseFloat(input) > 0) which does the comparison inside
Number() and always yields 0 or 1 incorrectly; change it so the numeric
conversion happens first and the comparison is outside, e.g. parseFloat(input) >
0 (or Number(parseFloat(input)) > 0) and optionally guard with
!isNaN(parseFloat(input)) if you need to reject non-numeric input.
| export default function TokenPrice(props: TokenPriceProps): JSX.Element { | ||
| const { value } = props; | ||
| const fixed = value.toFixed(10); | ||
| const parts = fixed.split('.'); | ||
|
|
There was a problem hiding this comment.
Guard against NaN/Infinity before calling toFixed/split
If value is NaN or Infinity, this will either throw (Infinity) or produce "NaN" and crash at decimals.search(...). Add a finite check before formatting.
export default function TokenPrice(props: TokenPriceProps): JSX.Element {
const { value } = props;
+ // Avoid runtime errors when value is NaN/Infinity
+ if (!Number.isFinite(value)) {
+ return <p style={{ fontSize: 13, fontWeight: 400 }}>-</p>;
+ }
const fixed = value.toFixed(10);
const parts = fixed.split('.');🤖 Prompt for AI Agents
In src/apps/pulse/components/Price/TokenPrice.tsx around lines 5-9, guard
against value being NaN or Infinity before calling value.toFixed and splitting:
detect non-finite values with Number.isFinite(value) (or
isFinite(Number(value))) and return a safe fallback (e.g., render '-' or 0) or
handle it appropriately; only call toFixed and split when the value is finite so
decimals.search won't crash.
| const getStatusIndex = ( | ||
| status: | ||
| | 'PENDING' | ||
| | 'SHORTLISTING_INITIATED' | ||
| | 'SHORTLISTED' | ||
| | 'EXECUTED' | ||
| | 'CLAIMED' | ||
| | 'RESOURCE_LOCK_RELEASED' | ||
| ) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify and type-safe status handling (include terminal/failure states)
Define a single BidStatus type and reuse it. This removes unsound casts and ensures failure states are handled consistently.
-const getStatusIndex = (
- status:
- | 'PENDING'
- | 'SHORTLISTING_INITIATED'
- | 'SHORTLISTED'
- | 'EXECUTED'
- | 'CLAIMED'
- | 'RESOURCE_LOCK_RELEASED'
-) => {
+type BidStatus =
+ | 'PENDING'
+ | 'SHORTLISTING_INITIATED'
+ | 'SHORTLISTED'
+ | 'EXECUTED'
+ | 'CLAIMED'
+ | 'RESOURCE_LOCK_RELEASED'
+ | 'SHORTLISTING_FAILED'
+ | 'FAILED_EXECUTION';
+
+const getStatusIndex = (status: BidStatus) => {📝 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.
| const getStatusIndex = ( | |
| status: | |
| | 'PENDING' | |
| | 'SHORTLISTING_INITIATED' | |
| | 'SHORTLISTED' | |
| | 'EXECUTED' | |
| | 'CLAIMED' | |
| | 'RESOURCE_LOCK_RELEASED' | |
| ) => { | |
| // Define a reusable, type-safe union for all bid statuses, including failure states. | |
| type BidStatus = | |
| | 'PENDING' | |
| | 'SHORTLISTING_INITIATED' | |
| | 'SHORTLISTED' | |
| | 'EXECUTED' | |
| | 'CLAIMED' | |
| | 'RESOURCE_LOCK_RELEASED' | |
| | 'SHORTLISTING_FAILED' | |
| | 'FAILED_EXECUTION'; | |
| const getStatusIndex = (status: BidStatus) => { | |
| // … existing implementation … | |
| } |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 17-25,
replace the inline union literal used for status with a single exported
BidStatus type (including terminal/failure states like FAILED, CANCELLED,
TIMED_OUT as applicable), then reuse that type across the file instead of
casting; update getStatusIndex signature to accept BidStatus, update any
callers/handlers to use the new type, remove unsound casts, and add an
exhaustive switch or default branch to handle unknown/terminal states so all
status values are type-checked and handled consistently.
| const [bid, setBid] = useState<any>(null); | ||
| const [resourceLockInfo, setResourceLockInfo] = useState<any>(null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any with minimal structural types (then drop ESLint disable)
Using any here hides real issues (you’re already branching on specific shapes). Define minimal structural types to cover what you actually read from API responses.
- const [bid, setBid] = useState<any>(null);
- const [resourceLockInfo, setResourceLockInfo] = useState<any>(null);
+ const [bid, setBid] = useState<BidState | null>(null);
+ const [resourceLockInfo, setResourceLockInfo] = useState<ResourceLockState | null>(null);Add these supporting types near the status helpers:
type ExecutionResult = { executedTransactions?: Array<{ transactionHash?: string }> };
type BidState = { bidStatus?: BidStatus; executionResult?: ExecutionResult };
type ResourceLockState = { resourceLocks?: Array<{ chainId?: number; transactionHash?: string }> };Once applied, you can safely remove the file-level eslint-disable for no-explicit-any.
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 38-39, the
useState calls currently use any which hides type issues — define the minimal
structural types (ExecutionResult, BidState, ResourceLockState) near the status
helpers, update the useState generics to use BidState and ResourceLockState
instead of any, and remove the file-level eslint-disable for no-explicit-any;
those types should at least include the fields you read (executedTransactions →
transactionHash, bidStatus, resourceLocks → chainId and transactionHash) so
existing property accesses type-check.
| const getCircleCss = ( | ||
| status: 'PENDING' | 'SHORTLISTING_INITIATED' | 'SHORTLISTED' | 'EXECUTED', | ||
| f = true | ||
| ) => { | ||
| const index = getStatusIndex(status); | ||
| if (f) { | ||
| if (resourceLockInfo?.resourceLocks?.[0]?.transactionHash) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use BidStatus in getCircleCss and rename ambiguous param
Align types with getStatusIndex and make the boolean parameter self-explanatory.
- const getCircleCss = (
- status: 'PENDING' | 'SHORTLISTING_INITIATED' | 'SHORTLISTED' | 'EXECUTED',
- f = true
- ) => {
- const index = getStatusIndex(status);
- if (f) {
+ const getCircleCss = (status: BidStatus, isFirstStep = true) => {
+ const index = getStatusIndex(status);
+ if (isFirstStep) {📝 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.
| const getCircleCss = ( | |
| status: 'PENDING' | 'SHORTLISTING_INITIATED' | 'SHORTLISTED' | 'EXECUTED', | |
| f = true | |
| ) => { | |
| const index = getStatusIndex(status); | |
| if (f) { | |
| if (resourceLockInfo?.resourceLocks?.[0]?.transactionHash) { | |
| const getCircleCss = (status: BidStatus, isFirstStep = true) => { | |
| const index = getStatusIndex(status); | |
| if (isFirstStep) { | |
| if (resourceLockInfo?.resourceLocks?.[0]?.transactionHash) { |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 41 to 47,
replace the inline union type for status with the existing BidStatus type used
by getStatusIndex and rename the ambiguous boolean parameter f to a descriptive
name (e.g., includeResourceLock or checkResourceLock) with a default of true;
update the function signature, any imports to include BidStatus, and all call
sites inside this file so they pass/expect the new parameter name, and ensure
logic that references resourceLockInfo remains unchanged.
| <img | ||
| src={token.logo} | ||
| alt="Token Logo" | ||
| style={{ | ||
| borderRadius: 50, | ||
| height: 32, | ||
| width: 32, | ||
| marginLeft: 10, | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make token logo alt text descriptive
The logo conveys identity; prefer a meaningful alt tied to the token.
- alt="Token Logo"
+ alt={token?.name ? `${token.name} logo` : 'Token logo'}📝 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.
| <img | |
| src={token.logo} | |
| alt="Token Logo" | |
| style={{ | |
| borderRadius: 50, | |
| height: 32, | |
| width: 32, | |
| marginLeft: 10, | |
| }} | |
| /> | |
| <img | |
| src={token.logo} | |
| alt={token?.name ? `${token.name} logo` : 'Token logo'} | |
| style={{ | |
| borderRadius: 50, | |
| height: 32, | |
| width: 32, | |
| marginLeft: 10, | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 106 to 115,
the img alt is a generic "Token Logo"; change it to a descriptive value using
the token data (e.g., token.name or `${token.symbol} logo`) and ensure a
sensible fallback (empty string only if purely decorative). Update the alt prop
to use the descriptive string and keep the rest of the img props intact to
improve accessibility.
| <div | ||
| className="flex w-full h-full overflow-hidden rounded-full" | ||
| style={{ | ||
| width: 32, | ||
| height: 32, | ||
| borderRadius: 50, | ||
| marginLeft: 10, | ||
| }} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Absolute overlay needs a positioned parent; center initials over the avatar
Without a positioned ancestor, the absolute span may not align reliably. Also center it without magic margins.
- <div
- className="flex w-full h-full overflow-hidden rounded-full"
+ <div
+ className="relative flex w-full h-full overflow-hidden rounded-full"
style={{
width: 32,
height: 32,
borderRadius: 50,
marginLeft: 10,
}}
>
<RandomAvatar name={token?.name || ''} />
- <span
- className="absolute flex items-center justify-center text-white text-lg font-bold"
- style={{ marginLeft: 5 }}
- >
+ <span
+ className="absolute inset-0 flex items-center justify-center text-white text-lg font-bold"
+ >Also applies to: 127-132
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 117-125 (and
similarly 127-132), the avatar wrapper is missing a positioned ancestor so the
absolute span for initials won't reliably align; change the wrapper to be
position: relative (via style or class), remove the hard-coded marginLeft used
to nudge the initials, and center the initials either by making the wrapper a
flex container with align-items:center and justify-content:center or by using
absolute positioning on the span with inset:0 and margin:auto (or
top:50%/left:50% with transform: translate(-50%,-50%)). Ensure the same fix is
applied to both blocks mentioned.
| chainId={resourceLockInfo?.resourceLocks?.[0]?.chainId} | ||
| completed={ | ||
| resourceLockInfo?.resourceLocks?.[0]?.transactionHash | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prop type: ensure completed is a boolean
If TransactionStatus.completed is boolean, passing a string hash is type-unsafe and unclear. Coerce to boolean.
- completed={
- resourceLockInfo?.resourceLocks?.[0]?.transactionHash
- }
+ completed={Boolean(
+ resourceLockInfo?.resourceLocks?.[0]?.transactionHash
+ )}🏁 Script executed:
#!/bin/bash
set -e
echo "1) Locate TransactionStatus component definition"
rg --color=never -n "export (const|function) TransactionStatus" -g "*.tsx" src/apps/pulse/components/Status
echo "2) Extract the TransactionStatusProps interface"
rg --color=never -A5 -B5 "interface TransactionStatusProps" -g "*.tsx" src/apps/pulse/components/Status
echo "3) Inspect the type of the completed prop"
rg --color=never -A2 -B2 "completed" -g "*.tsx" src/apps/pulse/components/StatusLength of output: 225
🏁 Script executed:
#!/bin/bash
set -e
echo "1) List TSX files in Status directory:"
ls src/apps/pulse/components/Status/*.tsx || true
echo
echo "2) Search for TransactionStatus usage in those files:"
rg -n "TransactionStatus" src/apps/pulse/components/Status/*.tsx || true
echo
echo "3) Search for the props definition (interface or type) for TransactionStatus:"
rg -n "TransactionStatusProps" -g "*.ts*" src/apps/pulse/components/Status || true
echo
echo "4) If a file defines TransactionStatusProps, show the completed field's type:"
FILE=$(rg -l "TransactionStatusProps" -g "*.ts*" src/apps/pulse/components/Status) || true
if [ -n "$FILE" ]; then
echo "Found in $FILE:"
rg -A3 -B3 "completed" "$FILE"
else
echo "No TransactionStatusProps definition found."
fiLength of output: 2497
Ensure completed prop is a boolean
The TransactionStatusProps type defines completed: boolean. Passing the raw transactionHash (a string) is type-unsafe. Coerce it to a boolean:
• File: src/apps/pulse/components/Status/IntentTracker.tsx
• Lines: ~168–171
- completed={
- resourceLockInfo?.resourceLocks?.[0]?.transactionHash
- }
+ completed={Boolean(
+ resourceLockInfo?.resourceLocks?.[0]?.transactionHash
+ )}📝 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.
| chainId={resourceLockInfo?.resourceLocks?.[0]?.chainId} | |
| completed={ | |
| resourceLockInfo?.resourceLocks?.[0]?.transactionHash | |
| } | |
| chainId={resourceLockInfo?.resourceLocks?.[0]?.chainId} | |
| completed={Boolean( | |
| resourceLockInfo?.resourceLocks?.[0]?.transactionHash | |
| )} |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 168 to 171,
the completed prop is being passed a transactionHash string but
TransactionStatusProps expects a boolean; change the prop to a boolean by
coercing the value (e.g.
!!resourceLockInfo?.resourceLocks?.[0]?.transactionHash) so completed receives
true when a hash exists and false otherwise, and adjust any local typing if
needed to keep TypeScript satisfied.
| backgroundColor: | ||
| getStatusIndex(bid?.bidStatus) < 3 ? '#121116' : '#8A77FF', | ||
| }} | ||
| disabled={getStatusIndex(bid?.bidStatus) < 3} | ||
| onClick={closePreview} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Allow Close when a terminal failure occurs; keep disabled otherwise
As-is, the bottom Close stays disabled on failures, while the Esc button remains enabled (inconsistent UX). Treat failure statuses as terminal so users can dismiss.
- backgroundColor:
- getStatusIndex(bid?.bidStatus) < 3 ? '#121116' : '#8A77FF',
+ backgroundColor:
+ (getStatusIndex(bid?.bidStatus) < 3 &&
+ !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(
+ bid?.bidStatus,
+ ))
+ ? '#121116'
+ : '#8A77FF',
}}
- disabled={getStatusIndex(bid?.bidStatus) < 3}
+ disabled={
+ getStatusIndex(bid?.bidStatus) < 3 &&
+ !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(
+ bid?.bidStatus,
+ )
+ }Would you prefer to also gate the top Esc button for consistency, or is it intended to always allow dismiss?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the closePreview button and its surrounding props
rg -n "closePreview" -A3 -B3 src/apps/pulse/components/Status/IntentTracker.tsx
# Check for any Escape key or onKeyDown handlers in the same file
rg -n -e "onKeyDown" -e "handleKey" -e "Escape" -A3 -B3 src/apps/pulse/components/Status/IntentTracker.tsxLength of output: 1169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate definition of getStatusIndex in the repository
rg -n "getStatusIndex" -A5 -B5 .
# Inspect mapping of statuses within getStatusIndex
rg -n "switch.*bidStatus" -A10 -B2 .Length of output: 4964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print getStatusIndex function definition and body
sed -n '1,100p' src/apps/pulse/components/Status/IntentTracker.tsx | sed -n '17,60p'Length of output: 1316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for other failure statuses used in the codebase
rg -n "FAILED_EXECUTION" -A2 -B2 .
rg -n "SHORTLISTING_FAILED" -A2 -B2 .
rg -n "FAILED_" -n .Length of output: 1185
Enable “Close” on terminal failures and keep disabled only for in-progress statuses
The Close button currently remains disabled for failure statuses because getStatusIndex returns –1 (<3). We should treat failures as terminal—enabling the button and using the active background—while still disabling for PENDING/SHORTLISTING_INITIATED/SHORTLISTED.
• File: src/apps/pulse/components/Status/IntentTracker.tsx (around lines 251–255)
--- a/src/apps/pulse/components/Status/IntentTracker.tsx
+++ b/src/apps/pulse/components/Status/IntentTracker.tsx
@@ -251,7 +251,15 @@ export default function IntentTracker(props: IntentTrackerProps) {
style={{
margin: 2,
borderRadius: 10,
- backgroundColor: getStatusIndex(bid?.bidStatus) < 3 ? '#121116' : '#8A77FF',
+ backgroundColor:
+ getStatusIndex(bid?.bidStatus) < 3 &&
+ !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(bid?.bidStatus)
+ ? '#121116'
+ : '#8A77FF',
}}
- disabled={getStatusIndex(bid?.bidStatus) < 3}
+ disabled={
+ getStatusIndex(bid?.bidStatus) < 3 &&
+ !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(bid?.bidStatus)
+ }
onClick={closePreview}
type="button"
>Also—should the top <Esc closePreview={...} /> be gated the same way for consistency, or is it intended to always allow dismissal?
📝 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.
| backgroundColor: | |
| getStatusIndex(bid?.bidStatus) < 3 ? '#121116' : '#8A77FF', | |
| }} | |
| disabled={getStatusIndex(bid?.bidStatus) < 3} | |
| onClick={closePreview} | |
| style={{ | |
| margin: 2, | |
| borderRadius: 10, | |
| backgroundColor: | |
| getStatusIndex(bid?.bidStatus) < 3 && | |
| !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(bid?.bidStatus) | |
| ? '#121116' | |
| : '#8A77FF', | |
| }} | |
| disabled={ | |
| getStatusIndex(bid?.bidStatus) < 3 && | |
| !['FAILED_EXECUTION', 'SHORTLISTING_FAILED'].includes(bid?.bidStatus) | |
| } | |
| onClick={closePreview} | |
| type="button" | |
| > |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Status/IntentTracker.tsx around lines 251–255, the
Close button treats failure statuses (getStatusIndex === -1) as in-progress
because the current check uses < 3; change the logic to only disable for
explicit in-progress indices 0,1,2 so failures are enabled and use the active
background: compute const idx = getStatusIndex(bid?.bidStatus); const disabled =
idx >= 0 && idx < 3; set backgroundColor based on that disabled flag; also make
the top <Esc closePreview={...} /> follow the same gating for consistency (or
explicitly keep it always enabled if intended) and apply the same idx-based
condition when deciding whether Esc should call closePreview.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/apps/pulse/hooks/useIntentSdk.ts (2)
21-24: Avoid defaulting bundlerApiKey to an empty string and confirm option completeness (mode/chain)
- Defaulting to '' masks configuration issues and may fail later at runtime. Use the raw value and guard for emptiness before initialization.
- Double-check whether IntentSdk Options should include network selector (e.g., mode or chain) for your deployment target.
Apply this minimal diff to avoid the empty default if you prefer not to adopt the larger refactor yet:
- bundlerApiKey: import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY || '', + bundlerApiKey: import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY,Would you like me to confirm the exact Options shape and required fields?
What are the supported constructor Options for @etherspot/intent-sdk IntentSdk? Is there a mode (testnet/mainnet) or chain parameter that should be provided alongside bundlerApiKey and modularAccount?
19-36: Harden IntentSdk initialization: validate env, add error handling, prevent stale updates, and cleanup on unmountThis consolidates earlier feedback and prevents unhandled promise rejections, stale state updates, and lingering SDK instances when auth/wallet changes. Also ensures we don’t proceed without a bundler key. This addresses earlier feedback about adding error handling and env validation.
Apply this diff:
- useEffect(() => { - if (accountAddress && ready && authenticated && walletProvider) { - const options: Options = { - bundlerApiKey: import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY || '', - modularAccount: accountAddress as Hex, - }; - - walletProvider.getEthereumProvider().then((provider) => { - const walletClient = createWalletClient({ - account: walletProvider.address as Hex, - transport: custom(provider), - }); - /* eslint-disable @typescript-eslint/no-explicit-any */ - const sdk = new IntentSdk(walletClient as any, options); - setIntentSdk(sdk); - }); - } - }, [accountAddress, ready, authenticated, walletProvider]); + useEffect(() => { + let disposed = false; + + const init = async () => { + if (!accountAddress || !ready || !authenticated || !walletProvider) { + setIntentSdk(null); + return; + } + + const bundlerApiKey = import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY; + if (!bundlerApiKey) { + console.error('VITE_ETHERSPOT_BUNDLER_API_KEY is not set'); + setIntentSdk(null); + return; + } + + try { + const provider = await walletProvider.getEthereumProvider(); + const walletClient = createWalletClient({ + account: walletProvider.address as Hex, + transport: custom(provider), + }); + + type WalletClientArg = ConstructorParameters<typeof IntentSdk>[0]; + const options: Options = { + bundlerApiKey, + modularAccount: accountAddress as Hex, + }; + + const sdk = new IntentSdk( + walletClient as unknown as WalletClientArg, + options + ); + if (!disposed) setIntentSdk(sdk); + } catch (error) { + if (!disposed) { + console.error('Failed to initialize IntentSdk:', error); + setIntentSdk(null); + } + } + }; + + init(); + return () => { + disposed = true; + setIntentSdk(null); + }; + }, [accountAddress, ready, authenticated, walletProvider]);
🧹 Nitpick comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
31-33: Don’t disable ESLint for the remainder of the file; scope it or remove “any” altogetherThe file-wide disable from Line 31 affects all subsequent lines. Either scope it to a single line or remove “any” by deriving the expected type from the IntentSdk constructor.
Minimal change:
- /* eslint-disable @typescript-eslint/no-explicit-any */ + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */Type-safe alternative (no ESLint disable):
type WalletClientArg = ConstructorParameters<typeof IntentSdk>[0]; const sdk = new IntentSdk(walletClient as unknown as WalletClientArg, options);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.eslintrc.json(1 hunks)package.json(3 hunks)src/apps/pulse/hooks/useIntentSdk.ts(1 hunks)src/apps/pulse/hooks/useModularSdk.ts(1 hunks)vite.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/apps/pulse/hooks/useModularSdk.ts
⏰ 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). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pulse
🔇 Additional comments (3)
src/apps/pulse/hooks/useIntentSdk.ts (2)
20-30: Verify walletClient.account aligns with modularAccount to avoid cross-account side effectsYou’re signing with walletProvider.address (EOA) while targeting modularAccount = accountAddress. If these don’t correspond (e.g., EOA is not an owner/controller of the modular account), SDK calls may fail or act on the wrong account. Please verify the mapping.
Suggested checks:
- Ensure walletProvider.address is the EOA that owns/controls the modular account at accountAddress.
- If multiple wallets exist, assert the correct pairing before initializing the SDK.
- Consider asserting at runtime and logging a warning if they differ unexpectedly.
If desired, I can add a guard that compares addresses or validates ownership via an SDK call before proceeding.
1-11: Solid hook composition and dependency gatingGood use of useWalletAddress/usePrivy/useWallets and dependency-based initialization. With the added guards/error handling, this should behave predictably across auth/wallet transitions.
.eslintrc.json (1)
105-106: Update ESLint verification stepsThe
--print-configcommand failed because ESLint v9 isn’t picking up your.eslintrc.jsonby default. To confirm that your negatedignorePatternsare actually re-includingsrc/apps/pulse/**, please rerun the check with an explicit config and/or without ignore patterns:
- Specify your JSON config:
npx eslint \ --config .eslintrc.json \ --print-config src/apps/pulse/index.tsx \ >/dev/null && echo "Pulse included ✅" || echo "Pulse excluded ❌"- Or test inclusion directly (ignoring ignore patterns):
npx eslint \ --config .eslintrc.json \ --no-ignore \ src/apps/pulse/index.tsx- If you’ve fully migrated to the new flat config format, ensure you’ve renamed/migrated to
eslint.config.jsor enabled the legacy loader plugin per the migration guide.Once you confirm that Pulse files produce a valid ESLint config or lint errors, you’ll know the negated patterns are working as intended.
| "public/", // Ignore public folder | ||
| "vite.config.js" | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t ignore vite.config.js; lint it with a Node-specific override instead.
Ignoring the Vite config can mask issues (e.g., typos in aliases, invalid plugin config). Prefer an override that enables Node env and proper parser options for this file.
Apply this diff to stop ignoring vite.config.js:
"ignorePatterns": [
"node_modules/",
"build/",
"postcss.config.js",
"config-overrides.js",
"tailwind.config.js",
"setupTests.ts",
"src/apps/**", // Ignore everything in src/apps/
"!src/apps/pillarx-app/", // Unignore this folder
"!src/apps/pillarx-app/**", // Unignore all contents in this folder
"!src/apps/the-exchange/",
"!src/apps/the-exchange/**",
"!src/apps/token-atlas/",
"!src/apps/token-atlas/**",
"!src/apps/deposit/",
"!src/apps/deposit/**",
"!src/apps/leaderboard/",
"!src/apps/leaderboard/**",
"!src/apps/pulse/",
"!src/apps/pulse/**",
- "public/", // Ignore public folder
- "vite.config.js"
+ "public/" // Ignore public folder
],Then (outside the selected lines) add a new override to lint Vite config correctly:
{
"overrides": [
{
"env": { "node": true },
"files": ["vite.config.{js,ts}"],
"parserOptions": { "sourceType": "module" }
}
]
}Optional: If you also have vitest config files, include ["vitest.config.{js,ts}"] similarly.
🤖 Prompt for AI Agents
.eslintrc.json around lines 107-109 currently ignores "vite.config.js"; remove
that entry from the "ignorePatterns" so the file is linted, and then add an
override entry in .eslintrc.json that targets "vite.config.{js,ts}" (and
optionally "vitest.config.{js,ts}") with env: { node: true } and parserOptions:
{ sourceType: "module" } so ESLint runs with a Node-specific configuration for
Vite config files.
| const walletProvider = wallets.find( | ||
| (wallet) => wallet.address === privyWalletAddress | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make address comparison case-insensitive to avoid provider lookup failures
Ethereum addresses are case-insensitive; strict string equality can fail if checksum casing differs, causing walletProvider to be undefined intermittently.
Apply this diff:
- const walletProvider = wallets.find(
- (wallet) => wallet.address === privyWalletAddress
- );
+ const walletProvider = wallets.find(
+ (wallet) => wallet.address?.toLowerCase() === privyWalletAddress?.toLowerCase()
+ );📝 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.
| const walletProvider = wallets.find( | |
| (wallet) => wallet.address === privyWalletAddress | |
| ); | |
| const walletProvider = wallets.find( | |
| (wallet) => wallet.address?.toLowerCase() === privyWalletAddress?.toLowerCase() | |
| ); |
🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 14 to 16, the wallet lookup
uses strict string equality which can fail due to Ethereum address checksum
casing; change the comparison to be case-insensitive by normalizing both
addresses before comparing (e.g., convert both to lowercase or normalize via a
canonical address helper), and guard against null/undefined addresses before
calling normalize so walletProvider lookup reliably finds the matching provider.
| 'crypto': 'crypto-browserify' | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Alias 'crypto' alone is insufficient; add missing polyfills and the 'node:crypto' alias to avoid runtime/prebundle failures.
Many browserified crypto paths also require stream, buffer, and process shims. Additionally, dependencies may import 'node:crypto'. Without these, Vite’s pre-bundling can fail or the app can crash at runtime.
Apply this minimal diff to extend aliases:
resolve: {
alias: {
'@': path.join(__dirname, 'src/apps'),
- 'crypto': 'crypto-browserify'
+ crypto: 'crypto-browserify',
+ 'node:crypto': 'crypto-browserify',
+ stream: 'stream-browserify',
+ buffer: 'buffer',
+ process: 'process/browser'
},
},And (outside the shown lines) add top-level define mappings to satisfy globals during dev/pre-bundle:
export default defineConfig({
plugins: [react(), svgr({}), dynamicImport()],
+ define: {
+ global: 'globalThis',
+ 'process.env': {},
+ },
build: {
outDir: 'build',
commonjsOptions: { transformMixedEsModules: true },
},Optional but recommended for smoother pre-bundling:
export default defineConfig({
plugins: [react(), svgr({}), dynamicImport()],
define: {
global: 'globalThis',
'process.env': {},
},
+ optimizeDeps: {
+ include: ['crypto-browserify', 'stream-browserify', 'buffer', 'process'],
+ esbuildOptions: {
+ define: {
+ global: 'globalThis',
+ 'process.env': '{}',
+ },
+ },
+ },Verification checklist to run locally:
- Ensure imports using both 'crypto' and 'node:crypto' resolve.
- Confirm no runtime ReferenceError for Buffer/global/process.
- Confirm dev server pre-bundle succeeds without warnings about unresolved stream/buffer/process.
🏁 Script executed:
#!/bin/bash
set -e
# Locate vite.config.js
CONFIG=$(fd --hidden --max-depth 2 vite.config.js || echo "")
if [[ -z "$CONFIG" ]]; then
echo "vite.config.js not found"
exit 1
fi
echo "Found vite.config.js at: $CONFIG"
echo "--- First 200 lines of vite.config.js ---"
sed -n '1,200p' "$CONFIG"Length of output: 986
Extend Vite config to fully polyfill Node.js built-ins for crypto & streams
The existing alias for 'crypto' alone will leave imports of node:crypto, stream, buffer, and process unresolved at build-time or runtime. You should also inject the necessary globals via define and (optionally) pre-bundle these polyfills in optimizeDeps.
Apply the following updates to vite.config.js:
• In resolve.alias, add missing aliases for
– node:crypto → crypto-browserify
– stream → stream-browserify
– buffer → buffer
– process → process/browser
• At the top level of defineConfig, add a define block:
– global: 'globalThis'
– 'process.env': {}
• (Optional) Under optimizeDeps, include the polyfill packages and re-define the globals for esbuild.
export default defineConfig({
plugins: [react(), svgr({}), dynamicImport()],
+ define: {
+ global: 'globalThis',
+ 'process.env': {},
+ },
build: {
outDir: 'build',
commonjsOptions: { transformMixedEsModules: true },
},
resolve: {
alias: {
'@': path.join(__dirname, 'src/apps'),
- 'crypto': 'crypto-browserify'
+ 'crypto': 'crypto-browserify',
+ 'node:crypto': 'crypto-browserify',
+ 'stream': 'stream-browserify',
+ 'buffer': 'buffer',
+ 'process': 'process/browser',
},
},
+ // Optional: ensure these polyfills are pre-bundled without warnings
+ optimizeDeps: {
+ include: ['crypto-browserify', 'stream-browserify', 'buffer', 'process'],
+ esbuildOptions: {
+ define: {
+ global: 'globalThis',
+ 'process.env': '{}',
+ },
+ },
+ },
test: {
globals: true,
environment: 'jsdom',
setupFiles: './src/test-utils/setupTests.ts',
define: {
global: 'globalThis',
},
pool: 'forks',
},
});📝 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.
| 'crypto': 'crypto-browserify' | |
| }, | |
| // vite.config.js | |
| import { defineConfig } from 'vite' | |
| import react from '@vitejs/plugin-react' | |
| import svgr from 'vite-plugin-svgr' | |
| import dynamicImport from 'vite-plugin-dynamic-import' | |
| import path from 'path' | |
| export default defineConfig({ | |
| plugins: [react(), svgr({}), dynamicImport()], | |
| define: { | |
| global: 'globalThis', | |
| 'process.env': {}, | |
| }, | |
| build: { | |
| outDir: 'build', | |
| commonjsOptions: { transformMixedEsModules: true }, | |
| }, | |
| resolve: { | |
| alias: { | |
| '@': path.join(__dirname, 'src/apps'), | |
| 'crypto': 'crypto-browserify', | |
| 'node:crypto': 'crypto-browserify', | |
| 'stream': 'stream-browserify', | |
| 'buffer': 'buffer', | |
| 'process': 'process/browser', | |
| }, | |
| }, | |
| // Optional: ensure these polyfills are pre-bundled without warnings | |
| optimizeDeps: { | |
| include: ['crypto-browserify', 'stream-browserify', 'buffer', 'process'], | |
| esbuildOptions: { | |
| define: { | |
| global: 'globalThis', | |
| 'process.env': '{}', | |
| }, | |
| }, | |
| }, | |
| test: { | |
| globals: true, | |
| environment: 'jsdom', | |
| setupFiles: './src/test-utils/setupTests.ts', | |
| define: { | |
| global: 'globalThis', | |
| }, | |
| pool: 'forks', | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In vite.config.js around lines 16-17, the current resolve.alias only maps
'crypto' to 'crypto-browserify' which leaves imports like 'node:crypto',
'stream', 'buffer', and 'process' unresolved; update resolve.alias to also map
'node:crypto' -> 'crypto-browserify', 'stream' -> 'stream-browserify', 'buffer'
-> 'buffer', and 'process' -> 'process/browser'; additionally add a top-level
define block in defineConfig setting global: 'globalThis' and 'process.env': {};
optionally add an optimizeDeps section listing the polyfill packages
(crypto-browserify, stream-browserify, buffer, process) and repeat esbuild
define mappings for globals to ensure pre-bundling.
3 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Style