-
Couldn't load subscription status.
- Fork 9
feat: add default currencies into the component #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add default currencies into the component #309
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shared/utils/initCurrencyManager.ts (1)
78-86: LGTM! Consider adding error logging for missing tokens.The changes improve the function's flexibility by gracefully handling both custom and default currency scenarios. The use of Map for token lookup is efficient.
Consider adding logging for tokens that weren't found to help diagnose configuration issues:
const currencies = currenciesToUse - .map((id) => tokenMap.get(id)) + .map((id) => { + const token = tokenMap.get(id); + if (!token) { + console.warn(`Token not found in token list: ${id}`); + } + return token; + }) .filter((token): token is CurrencyTypes.CurrencyInput => token != null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shared/utils/initCurrencyManager.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
shared/utils/initCurrencyManager.ts (2)
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: shared/utils/currencyConversionPairs.ts:8-1164
Timestamp: 2024-11-18T03:44:52.936Z
Learning: Do not recommend adding a `const` enum or union type for network names in `shared/utils/currencyConversionPairs.ts`, since the data object mirrors the response from the price aggregators subgraph, minimizing the risk of typos.
Learnt from: aimensahnoun
PR: RequestNetwork/web-components#303
File: shared/utils/initCurrencyManager.ts:101-119
Timestamp: 2025-01-14T13:16:45.499Z
Learning: The Request Network token list at "https://requestnetwork.github.io/request-token-list/latest.json" is a maintained JSON file that serves as the source of truth for available tokens in the Request Network ecosystem. The `fetchTokenList` function reliably returns this comprehensive list of tokens.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
shared/utils/initCurrencyManager.ts (1)
5-32: Verify the impact of removed currencies on existing users.The significant reduction in supported default currencies (removing CHF, SGD, AUD, etc.) might affect users who rely on these currencies. While the remaining list focuses on major currencies and popular tokens, we should ensure this change is intentional and properly communicated.
Run this script to check for usage of removed currencies in the codebase:
✅ Verification successful
Removed currencies have no impact on existing codebase
The removal of currencies (CHF, SGD, AUD, etc.) from the default currency list appears to be safe as there are no direct references to these currencies in the codebase. The new list appropriately focuses on major fiat currencies and commonly used blockchain tokens.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of removed currencies to assess impact # List of removed currencies to check currencies=("CHF" "SGD" "AUD" "BRL" "CAD" "INR" "KRW" "IDR" "NZD" "TRY") echo "Searching for usage of removed currencies..." for currency in "${currencies[@]}"; do echo "Checking $currency..." rg -l "$currency" --type ts doneLength of output: 1225
Script:
#!/bin/bash # Check the context of AUD usage and search for currency-related files echo "=== Context of AUD usage ===" rg "AUD" -C 5 packages/payment-widget/src/lib/utils/currencies.ts echo -e "\n=== Finding currency-related files ===" fd -t f -e ts -e json "curr|currency|currencies" --exec echo "{}"Length of output: 778
Summary by CodeRabbit
New Features
Refactor