-
Notifications
You must be signed in to change notification settings - Fork 9
Refactor: Centralize URLs in a constant file #574
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
base: main
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughCentralizes many hardcoded URLs into a new constants/urls module and updates numerous modules to construct endpoints and asset links via those base URL constants using the URL API. Also renames a ShareButtons prop and adds an optional FAQContent prop; no control-flow changes. Changes
Sequence Diagram(s)(Skipped — changes are refactors/config centralization without new multi-component control-flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR centralizes hardcoded URLs into a single constants/urls.ts file and refactors URL construction throughout the codebase to use the URL constructor instead of string concatenation, improving maintainability and reducing the risk of malformed URLs.
Key changes:
- Created
constants/urls.tswith base URLs for all external services and platforms - Refactored URL construction to use
new URL()with base URLs and paths - Updated URL query parameter handling to use
searchParamsor direct search string assignment
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vechain-kit/src/constants/urls.ts | New centralized constants file containing all base URLs |
| packages/vechain-kit/src/constants/index.ts | Exports URL constants for consumption |
| packages/vechain-kit/src/utils/constants.tsx | Refactored to use URL constants and new URL() for construction |
| packages/vechain-kit/src/utils/swap/veTrade.tsx | Updated VeTrade API URL construction with centralized constant |
| packages/vechain-kit/src/utils/swap/apiAggregator.ts | Refactored query string handling to use URL object |
| packages/vechain-kit/src/providers/PrivyWalletProvider.tsx | Updated transaction endpoint URL construction |
| packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx | Replaced hardcoded node and explorer URLs with constants |
| packages/vechain-kit/src/hooks/utils/useAppHubApps.ts | Refactored GitHub API calls to use URL constants |
| packages/vechain-kit/src/hooks/login/useLoginWithVeChain.ts | Updated governance website URL with constant |
| packages/vechain-kit/src/hooks/generic-delegator/useGenericDelegator.ts | Refactored delegator API URLs to use URL constructor |
| packages/vechain-kit/src/hooks/api/vetDomains/*.ts | Updated node API endpoint construction |
| packages/vechain-kit/src/hooks/api/privy/*.ts | Refactored Privy API URLs with constants |
| packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsx | Updated social sharing URLs with constants and proper encoding |
| packages/vechain-kit/src/components/AccountModal/Contents//.tsx | Replaced hardcoded URLs throughout modal components |
| packages/vechain-kit/src/assets/icons/PrivyLogo/PrivyLogo.tsx | Updated Privy logo URLs with centralized constant |
Comments suppressed due to low confidence (1)
packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsx:1
- Corrected spelling of 'Mughsot' to 'Mugshot'.
import { Box, HStack, Link } from '@chakra-ui/react';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 1
🧹 Nitpick comments (2)
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (1)
46-54: Consider centralizing the iconUrl.The
iconUrlon line 50-51 is still hardcoded, butIMAGEDELIVERY_NET_BASE_URLwas imported on line 19. For consistency with the PR's centralization goal, this URL could be constructed using the imported constant.🔎 Suggested refactor
export const vechainConnector = () => { return toPrivyWalletConnector({ id: VECHAIN_PRIVY_APP_ID, name: 'VeChain', - iconUrl: - 'https://imagedelivery.net/oHBRUd2clqykxgDWmeAyLg/661dd77c-2f9d-40e7-baa1-f4e24fd7bf00/icon', + iconUrl: new URL( + '/oHBRUd2clqykxgDWmeAyLg/661dd77c-2f9d-40e7-baa1-f4e24fd7bf00/icon', + IMAGEDELIVERY_NET_BASE_URL + ).toString(), smartWalletMode: false, }); };packages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsx (1)
62-111: LGTM! URL construction is correct and consistent.The refactoring to use centralized base URL constants with the URL constructor is well-executed. The pattern is consistent across all entries in DEFAULT_APPS.
💡 Optional simplification
For
external_urlfields usingnew URL('/', BASE_URL).toString(), you could simplify to justBASE_URLsince the root path returns the base URL itself:- external_url: new URL('/', VEBETTERDAO_GOVERNANCE_BASE_URL).toString(), + external_url: VEBETTERDAO_GOVERNANCE_BASE_URL,However, maintaining the current pattern provides consistency with other URL constructions in this object, so this is purely optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/vechain-kit/src/assets/icons/BetterSwapLogo/BetterSwapLogo.tsxpackages/vechain-kit/src/assets/icons/PrivyLogo/PrivyLogo.tsxpackages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsxpackages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsxpackages/vechain-kit/src/constants/index.tspackages/vechain-kit/src/constants/urls.tspackages/vechain-kit/src/hooks/api/privy/useFetchAppInfo.tspackages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarLegacy.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.tspackages/vechain-kit/src/hooks/generic-delegator/useGenericDelegator.tspackages/vechain-kit/src/hooks/login/useLoginWithVeChain.tspackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/providers/PrivyWalletProvider.tsxpackages/vechain-kit/src/utils/constants.tsxpackages/vechain-kit/src/utils/swap/apiAggregator.tspackages/vechain-kit/src/utils/swap/veTrade.tsx
💤 Files with no reviewable changes (1)
- packages/vechain-kit/src/assets/icons/BetterSwapLogo/BetterSwapLogo.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/migration-guide-to-v2.mdc)
**/*.{ts,tsx}: In VeChain Kit Version 2, useuseThorinstead ofuseConnexfor contract interactions
For single contract read operations, use theuseCallClausehook with the pattern: import dependencies, define ABI and method as const, create query key function usinggetCallClauseQueryKeyWithArgs, and wrap with useCallClause including data transformation in the select option
For multiple parallel contract calls, use theexecuteMultipleClausesCallutility wrapped in auseQueryhook with the pattern: define query key function, useexecuteMultipleClausesCallin queryFn mapping items to clause objects, and transform results
For transaction building and sending, use theuseBuildTransactionhook with a clauseBuilder function that returns an array of clauses with optional comment fields describing the transaction action
Always provide an arguments array for contract calls, even when no parameters are required - use an empty array for parameter-less functions to enable TypeScript type checking
Always conditionally enable queries using theenabledproperty to prevent unnecessary contract calls, checking for all required parameters:enabled: !!requiredParam && !!otherRequiredParam
Use theselectoption in useCallClause or transform data in queryFn to handle data transformation, particularly for converting BigInt values to strings and normalizing contract return data
Maintain consistent query key patterns: usegetCallClauseQueryKeyWithArgsfor contract calls with arguments andgetCallClauseQueryKeyfor calls without arguments to ensure proper caching and invalidation
Use TypeScriptas constassertions for method names andas0x${string}`` assertions for Ethereum addresses to ensure type safety in contract interactions
Files:
packages/vechain-kit/src/constants/index.tspackages/vechain-kit/src/utils/swap/apiAggregator.tspackages/vechain-kit/src/assets/icons/PrivyLogo/PrivyLogo.tsxpackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarLegacy.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.tspackages/vechain-kit/src/hooks/login/useLoginWithVeChain.tspackages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsxpackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.tspackages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.tspackages/vechain-kit/src/providers/PrivyWalletProvider.tsxpackages/vechain-kit/src/hooks/api/privy/useFetchAppInfo.tspackages/vechain-kit/src/utils/swap/veTrade.tsxpackages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsxpackages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsxpackages/vechain-kit/src/hooks/generic-delegator/useGenericDelegator.tspackages/vechain-kit/src/utils/constants.tsxpackages/vechain-kit/src/constants/urls.ts
🧠 Learnings (1)
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : In VeChain Kit Version 2, use `useThor` instead of `useConnex` for contract interactions
Applied to files:
packages/vechain-kit/src/constants/index.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.tspackages/vechain-kit/src/hooks/login/useLoginWithVeChain.tspackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/providers/PrivyWalletProvider.tsxpackages/vechain-kit/src/utils/swap/veTrade.tsxpackages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsxpackages/vechain-kit/src/utils/constants.tsx
🧬 Code graph analysis (14)
packages/vechain-kit/src/assets/icons/PrivyLogo/PrivyLogo.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
PRIVY_MINTLIFY_ASSETS_S3_BASE_URL(76-77)
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (1)
packages/vechain-kit/src/constants/urls.ts (3)
VECHAIN_MAINNET_NODE_BASE_URL(15-15)VECHAIN_EXPLORER_BASE_URL(43-43)VECHAINSTATS_BASE_URL(45-45)
packages/vechain-kit/src/hooks/login/useLoginWithVeChain.ts (1)
packages/vechain-kit/src/constants/urls.ts (1)
VEBETTERDAO_GOVERNANCE_BASE_URL(48-48)
packages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsx (1)
packages/vechain-kit/src/constants/urls.ts (3)
VEBETTERDAO_GOVERNANCE_BASE_URL(48-48)IBB_CO_BASE_URL(73-73)VET_DOMAINS_BASE_URL(35-35)
packages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
VECHAIN_KIT_DOCS_BASE_URL(19-19)
packages/vechain-kit/src/hooks/utils/useAppHubApps.ts (1)
packages/vechain-kit/src/constants/urls.ts (2)
APP_HUB_GITHUB_API_BASE_URL(4-4)APP_HUB_GITHUB_RAW_REPO_BASE_URL(6-7)
packages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.ts (1)
packages/vechain-kit/src/constants/urls.ts (1)
VECHAIN_KIT_DOCS_IMAGES_S3_BASE_URL(74-75)
packages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.ts (1)
packages/vechain-kit/src/constants/urls.ts (1)
PRIVY_STATUS_BASE_URL(11-11)
packages/vechain-kit/src/hooks/api/privy/useFetchAppInfo.ts (2)
packages/vechain-kit/src/types/types.ts (1)
PrivyAppInfo(132-147)packages/vechain-kit/src/constants/urls.ts (1)
PRIVY_AUTH_BASE_URL(12-12)
packages/vechain-kit/src/utils/swap/veTrade.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
VETRADE_BASE_URL(50-50)
packages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
VEBETTERDAO_GOVERNANCE_BASE_URL(48-48)
packages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
VECHAIN_ENERGY_SWAP_BASE_URL(49-49)
packages/vechain-kit/src/hooks/generic-delegator/useGenericDelegator.ts (2)
packages/vechain-kit/src/types/gasToken.ts (2)
GasTokenType(1-1)TransactionSpeed(3-3)packages/vechain-kit/src/types/gasEstimation.ts (1)
DepositAccount(45-47)
packages/vechain-kit/src/utils/constants.tsx (1)
packages/vechain-kit/src/constants/urls.ts (12)
CRYPTOLOGOS_BASE_URL(67-67)COINMARKETCAP_STATIC_BASE_URL(68-68)VECHAIN_TOKEN_REGISTRY_ASSETS_BASE_URL(65-66)APP_HUB_GITHUB_RAW_REPO_BASE_URL(6-7)IMAGE_NOT_FOUND_URL(69-70)GENERIC_DELEGATOR_MAINNET_URL(39-39)GENERIC_DELEGATOR_TESTNET_URL(40-40)CLEANIFY_APP_BASE_URL(53-53)GREENCART_BASE_URL(54-54)MUGSHOT_BASE_URL(55-55)EVEARN_BASE_URL(56-56)VECHAIN_KIT_WEBSITE_BASE_URL(18-18)
⏰ 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: e2e-test
- GitHub Check: Lint & Build
- GitHub Check: build-size-check
- GitHub Check: deploy
🔇 Additional comments (31)
packages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.ts (1)
31-32: LGTM!The URL construction using
new URL('/accounts/*', nodeUrl)correctly creates a reusable URL object that is shared between both fetch calls. This approach is cleaner than string interpolation and properly handles URL path resolution.packages/vechain-kit/src/assets/icons/PrivyLogo/PrivyLogo.tsx (1)
14-21: LGTM!The URL construction correctly uses the centralized base URL constant. The
new URL()constructor properly handles path resolution with the S3 base URL.packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsx (1)
38-47: Acknowledged pre-encoding contract.The direct assignment to
.searchcorrectly avoids double-encoding whendescriptionEncodedis already URI-encoded upstream. This approach is intentional per the inline comment. Consider adding a type or validation to enforce the pre-encoded contract if this becomes error-prone.packages/vechain-kit/src/constants/index.ts (1)
1-1: LGTM!Clean barrel export that centralizes all URL constants under a single import path.
packages/vechain-kit/src/utils/swap/apiAggregator.ts (1)
216-218: LGTM!The URL construction correctly uses the URL API to build the request URL. Passing the URL object directly to
fetch()is valid and cleaner than string conversion.packages/vechain-kit/src/hooks/login/useLoginWithVeChain.ts (1)
21-21: LGTM!The URL construction correctly uses the centralized constant. Using
new URL('/', BASE_URL)ensures a properly formatted base URL with trailing slash.packages/vechain-kit/src/hooks/api/privy/useFetchAppInfo.ts (1)
9-10: LGTM!The URL construction correctly uses template literal for the dynamic path segment with the centralized base URL constant. Passing the URL object directly to
fetch()is clean and valid.packages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsx (1)
116-119: LGTM!The URL construction correctly uses the centralized constant as a fallback, maintaining consistency with the same pattern used in
useLoginWithVeChain.ts.packages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.ts (1)
7-7: LGTM! Clean URL centralization.The import and URL construction pattern are correct. Using
new URL(path, base).toString()for the cross-app avatar map entries ensures consistent URL handling and makes the base URL configurable.Also applies to: 20-28
packages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsx (1)
19-19: LGTM! Proper URL construction for external link.The change correctly uses the centralized constant with URL constructor to build the VeChain Energy swap URL.
Also applies to: 32-34
packages/vechain-kit/src/providers/PrivyWalletProvider.tsx (1)
141-142: LGTM! Safer URL construction.Replacing string concatenation with the URL constructor is a good improvement. The
fetchAPI accepts URL objects directly, so no.toString()is needed here.packages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.ts (2)
2-2: LGTM! Consistent URL centralization.The URL construction follows the same pattern as other files in this PR, improving maintainability.
Also applies to: 6-7
16-16: Minor formatting improvement.The quote style change improves consistency across the codebase.
packages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarLegacy.ts (2)
58-59: LGTM! Good DRY improvement.Creating the URL object once and reusing it for multiple fetches is cleaner than the previous string concatenation approach.
Also applies to: 89-89
177-178: Consistent URL construction.The same URL object pattern is applied to the NFT metadata fetch logic.
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (1)
18-23: LGTM! Chain configuration properly centralized.The VeChain chain definition now uses centralized constants for RPC endpoints and block explorers, improving configurability.
Also applies to: 25-44
packages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsx (1)
23-23: LGTM! Docs URL properly centralized.The documentation link now uses the centralized constant with proper URL construction.
Also applies to: 83-83
packages/vechain-kit/src/utils/swap/veTrade.tsx (1)
11-11: LGTM! Centralized VeTrade API URL.The URL construction is correct. Keeping the
networkTypeparameter with a comment about future overrides is good forward-compatibility design.Also applies to: 42-46
packages/vechain-kit/src/hooks/utils/useAppHubApps.ts (2)
4-7: LGTM! Centralized base URL imports are clean.The imports from constants properly separate concerns and improve maintainability.
57-94: LGTM! URL construction with template literals is correct.The refactoring properly uses the URL constructor with:
- Absolute path for the GitHub API endpoint (Line 58)
- Template literals for dynamic paths with manifest and logo files (Lines 73, 86)
- Proper use of
toString()for the logo URL (Line 94)All URL constructions will produce the expected results.
packages/vechain-kit/src/hooks/generic-delegator/useGenericDelegator.ts (3)
17-43: LGTM! Proper use of URL constructor with searchParams.The refactoring correctly:
- Constructs the estimate URL with a relative path and template literal (Lines 24-26)
- Uses
searchParams.set()for query parameters instead of string concatenation (Lines 28-29)- Maintains the same request body structure
The relative path (without leading slash) properly appends to the base URL's existing path.
45-57: LGTM! URL construction for deposit account is correct.The relative path
'deposit/account'correctly appends to thegenericDelegatorUrlbase, which includes/api/v1/.
59-77: LGTM! Template literal in URL constructor works correctly.The URL construction with the template literal for the authorization endpoint is correct. The
token.toLowerCase()is properly included in both the URL path (Line 61) and the request body (Line 72).packages/vechain-kit/src/utils/constants.tsx (6)
11-24: LGTM! Comprehensive import of centralized URL constants.All required base URL constants are properly imported from the new constants module.
26-51: LGTM! TOKEN_LOGOS refactored to use URL constructors.The refactoring correctly constructs token logo URLs from centralized base URLs:
- VET, VTHO logos from external services
- B3TR, VOT3, veDelegate from VeChain token registry (using hash-based paths)
- USDGLO from GitHub raw content (using commit SHA path)
All URL constructions are correct and maintain the same final URLs as before.
62-62: LGTM! Centralized constant for fallback image.Using
IMAGE_NOT_FOUND_URLconstant improves maintainability.
105-110: LGTM! Generic delegator URL construction is cleaner.The refactoring uses centralized mainnet and testnet delegator URL constants, making it easier to update these endpoints in the future.
119-140: LGTM! Ecosystem apps now use centralized base URLs.All four apps (Cleanify, GreenCart, Mugshot, EVearn) properly construct their website URLs using the new base URL constants.
142-157: LGTM! Terms and cookies URLs properly centralized.Both configuration objects now construct their URLs from
VECHAIN_KIT_WEBSITE_BASE_URL, ensuring consistency if the base URL changes.packages/vechain-kit/src/constants/urls.ts (2)
1-87: LGTM! Well-organized centralized URL constants.The new constants file provides excellent organization of all base URLs used throughout the application:
- Logical grouping by service category
- Clear, descriptive constant names
- Helpful comments separating sections
- Comprehensive coverage of VeChain infrastructure, ecosystem apps, and external services
This centralization significantly improves maintainability and makes it easier to update endpoints in the future.
62-62: The URL is correct as-is and requires no change.
https://insight.vecha.inis the official VeChain Insight testnet domain. Thevecha.inshort domain is legitimate and widely documented in VeChain's official resources, including the VeChain documentation and ecosystem partners. The alternative suggestion ofvechain.inis not used for Insight services.Likely an incorrect or invalid review comment.
🚀 Preview environment deployed!Preview URL: https://preview.vechainkit.vechain.org/fixcentralize-urls |
|
Size Change: +17.8 kB (+0.31%) Total Size: 5.78 MB
ℹ️ View Unchanged
|
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 (2)
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (1)
49-50: Consider centralizing the iconUrl.While the VeChain-specific URLs have been centralized, this imagedelivery.net URL remains hardcoded. For consistency with the PR's objective to centralize URLs, consider moving this to the constants file as well.
packages/vechain-kit/src/utils/constants.tsx (1)
103-108: Consider using URL constructor for consistency.The function still uses string concatenation to append
/api/v1/to the base URLs. For consistency with the PR's objective and to improve URL handling, consider using theURLconstructor:🔎 Proposed refactor using URL constructor
export const getGenericDelegatorUrl = () => { const env = getENV(); return env.isProduction - ? `${GENERIC_DELEGATOR_MAINNET_URL}/api/v1/` - : `${GENERIC_DELEGATOR_TESTNET_URL}/api/v1/`; // or url to your delegator + ? new URL('/api/v1/', GENERIC_DELEGATOR_MAINNET_URL).toString() + : new URL('/api/v1/', GENERIC_DELEGATOR_TESTNET_URL).toString(); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsxpackages/vechain-kit/src/config/mainnet.tspackages/vechain-kit/src/config/testnet.tspackages/vechain-kit/src/constants/urls.tspackages/vechain-kit/src/hooks/login/useLoginWithVeChain.tspackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/utils/constants.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/vechain-kit/src/components/AccountModal/Contents/Ecosystem/ExploreEcosystemContent.tsx
- packages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsx
- packages/vechain-kit/src/components/AccountModal/Contents/Bridge/BridgeContent.tsx
- packages/vechain-kit/src/hooks/login/useLoginWithVeChain.ts
- packages/vechain-kit/src/hooks/utils/useAppHubApps.ts
- packages/vechain-kit/src/constants/urls.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/migration-guide-to-v2.mdc)
**/*.{ts,tsx}: In VeChain Kit Version 2, useuseThorinstead ofuseConnexfor contract interactions
For single contract read operations, use theuseCallClausehook with the pattern: import dependencies, define ABI and method as const, create query key function usinggetCallClauseQueryKeyWithArgs, and wrap with useCallClause including data transformation in the select option
For multiple parallel contract calls, use theexecuteMultipleClausesCallutility wrapped in auseQueryhook with the pattern: define query key function, useexecuteMultipleClausesCallin queryFn mapping items to clause objects, and transform results
For transaction building and sending, use theuseBuildTransactionhook with a clauseBuilder function that returns an array of clauses with optional comment fields describing the transaction action
Always provide an arguments array for contract calls, even when no parameters are required - use an empty array for parameter-less functions to enable TypeScript type checking
Always conditionally enable queries using theenabledproperty to prevent unnecessary contract calls, checking for all required parameters:enabled: !!requiredParam && !!otherRequiredParam
Use theselectoption in useCallClause or transform data in queryFn to handle data transformation, particularly for converting BigInt values to strings and normalizing contract return data
Maintain consistent query key patterns: usegetCallClauseQueryKeyWithArgsfor contract calls with arguments andgetCallClauseQueryKeyfor calls without arguments to ensure proper caching and invalidation
Use TypeScriptas constassertions for method names andas0x${string}`` assertions for Ethereum addresses to ensure type safety in contract interactions
Files:
packages/vechain-kit/src/config/mainnet.tspackages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsxpackages/vechain-kit/src/config/testnet.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/utils/constants.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : In VeChain Kit Version 2, use `useThor` instead of `useConnex` for contract interactions
Applied to files:
packages/vechain-kit/src/config/mainnet.tspackages/vechain-kit/src/providers/PrivyCrossAppProvider.tsxpackages/vechain-kit/src/utils/constants.tsx
🧬 Code graph analysis (3)
packages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsx (1)
packages/vechain-kit/src/constants/urls.ts (1)
VEBETTERDAO_GOVERNANCE_BASE_URL(44-45)
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (1)
packages/vechain-kit/src/constants/urls.ts (3)
VECHAIN_MAINNET_NODE_BASE_URL(14-14)VECHAIN_EXPLORER_BASE_URL(38-38)VECHAINSTATS_BASE_URL(41-41)
packages/vechain-kit/src/utils/constants.tsx (1)
packages/vechain-kit/src/constants/urls.ts (10)
COINMARKETCAP_STATIC_BASE_URL(61-61)VECHAIN_TOKEN_REGISTRY_ASSETS_BASE_URL(59-60)IMAGE_NOT_FOUND_URL(62-63)GENERIC_DELEGATOR_MAINNET_URL(32-33)GENERIC_DELEGATOR_TESTNET_URL(34-35)CLEANIFY_APP_BASE_URL(50-50)GREENCART_BASE_URL(51-51)MUGSHOT_BASE_URL(52-52)EVEARN_BASE_URL(53-53)VECHAIN_KIT_WEBSITE_BASE_URL(16-16)
🔇 Additional comments (10)
packages/vechain-kit/src/components/AccountModal/Components/CrossAppConnectionSecurityCard.tsx (1)
17-17: LGTM! URL successfully centralized.The refactoring correctly replaces the hardcoded URL with the imported constant
VEBETTERDAO_GOVERNANCE_BASE_URL. The fallback pattern is appropriate, and this change improves maintainability by centralizing URL management.Also applies to: 116-116
packages/vechain-kit/src/providers/PrivyCrossAppProvider.tsx (2)
18-22: LGTM! Clean import of centralized URL constants.The imports are well-structured and all constants are used appropriately in the file.
24-43: LGTM! Proper use of centralized URL constants.The VeChain chain definition correctly uses the centralized URL constants for RPC endpoints and block explorers. This improves maintainability and ensures consistency across the codebase.
packages/vechain-kit/src/utils/constants.tsx (5)
11-22: LGTM! Clean import organization.The centralized URL constants are properly imported and ready for use throughout the file.
117-138: LGTM! Excellent use of centralized constants.The ecosystem apps now reference the base URL constants directly, making them easier to maintain and update.
141-146: LGTM! Proper URL construction.The terms URL is now correctly constructed using the
URLconstructor with the centralized base URL constant.
150-155: LGTM! Proper URL construction.The cookies URL is now correctly constructed using the
URLconstructor with the centralized base URL constant.
25-48: No changes needed—VOT3 path format is correct.The VOT3 token intentionally uses a contract address format (
17ff70aa1d898bc97ad690dbfad1a3b5643f7e0b) rather than a CoinMarketCap ID, matching the pattern of veDelegate. This is appropriate because VOT3 is a VeChain native token, not a standard CoinMarketCap-listed token. Additionally, VOT3 includes a JSX component fallback (VOT3Logo), so the URL serves as a secondary source.packages/vechain-kit/src/config/mainnet.ts (1)
7-7: LGTM! Indexer URL updated to unified domain.The b3trIndexerUrl now points to the unified indexer service domain, which is consistent with the testnet configuration and improves maintainability.
packages/vechain-kit/src/config/testnet.ts (1)
48-48: LGTM! Testnet indexer URL aligned with mainnet.The b3trIndexerUrl now uses the unified indexer domain, consistent with the mainnet configuration. This ensures both environments use the same URL structure.
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: 1
♻️ Duplicate comments (1)
packages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.ts (1)
16-17: Generic error message lacks context.The error message 'Error fetching data:' doesn't indicate this is specifically a Privy status fetch failure, which could make debugging harder.
🔎 Suggested improvement
- console.error('Error fetching data:', error); - return 'Error fetching data'; + console.error('Error fetching Privy status:', error); + return 'Error fetching Privy status';
🧹 Nitpick comments (2)
packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsx (1)
29-33: Remove unused Props fields.The
urlandfacebookHashtagproperties are defined in the Props type but never used in the component. Consider removing them to reduce confusion and maintain a cleaner interface.🔎 Proposed cleanup
type Props = { description: string; - url?: string; - facebookHashtag?: string; };packages/vechain-kit/src/utils/constants.tsx (1)
103-108: Consider using URL constructor for consistency.The function uses string concatenation to build URLs, which is inconsistent with the URL constructor approach used throughout this PR. While functional, adopting the URL constructor pattern would improve consistency and reduce the risk of path-joining errors.
🔎 Proposed refactor using URL constructor
export const getGenericDelegatorUrl = () => { const env = getENV(); - return env.isProduction - ? `${GENERIC_DELEGATOR_MAINNET_URL}/api/v1/` - : `${GENERIC_DELEGATOR_TESTNET_URL}/api/v1/`; // or url to your delegator + const baseUrl = env.isProduction + ? GENERIC_DELEGATOR_MAINNET_URL + : GENERIC_DELEGATOR_TESTNET_URL; + return new URL('/api/v1/', baseUrl).toString(); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsxpackages/vechain-kit/src/constants/urls.tspackages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarLegacy.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.tspackages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.tspackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/providers/PrivyWalletProvider.tsxpackages/vechain-kit/src/utils/constants.tsxpackages/vechain-kit/src/utils/swap/veTrade.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/vechain-kit/src/utils/swap/veTrade.tsx
- packages/vechain-kit/src/hooks/api/vetDomains/useGetTextRecords.ts
- packages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarLegacy.ts
- packages/vechain-kit/src/providers/PrivyWalletProvider.tsx
- packages/vechain-kit/src/hooks/api/vetDomains/useGetAvatarOfAddress.ts
- packages/vechain-kit/src/constants/urls.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/migration-guide-to-v2.mdc)
**/*.{ts,tsx}: In VeChain Kit Version 2, useuseThorinstead ofuseConnexfor contract interactions
For single contract read operations, use theuseCallClausehook with the pattern: import dependencies, define ABI and method as const, create query key function usinggetCallClauseQueryKeyWithArgs, and wrap with useCallClause including data transformation in the select option
For multiple parallel contract calls, use theexecuteMultipleClausesCallutility wrapped in auseQueryhook with the pattern: define query key function, useexecuteMultipleClausesCallin queryFn mapping items to clause objects, and transform results
For transaction building and sending, use theuseBuildTransactionhook with a clauseBuilder function that returns an array of clauses with optional comment fields describing the transaction action
Always provide an arguments array for contract calls, even when no parameters are required - use an empty array for parameter-less functions to enable TypeScript type checking
Always conditionally enable queries using theenabledproperty to prevent unnecessary contract calls, checking for all required parameters:enabled: !!requiredParam && !!otherRequiredParam
Use theselectoption in useCallClause or transform data in queryFn to handle data transformation, particularly for converting BigInt values to strings and normalizing contract return data
Maintain consistent query key patterns: usegetCallClauseQueryKeyWithArgsfor contract calls with arguments andgetCallClauseQueryKeyfor calls without arguments to ensure proper caching and invalidation
Use TypeScriptas constassertions for method names andas0x${string}`` assertions for Ethereum addresses to ensure type safety in contract interactions
Files:
packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsxpackages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.tspackages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/utils/constants.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-01T13:01:33.771Z
Learnt from: CR
Repo: vechain/vechain-kit PR: 0
File: .cursor/rules/migration-guide-to-v2.mdc:0-0
Timestamp: 2025-12-01T13:01:33.771Z
Learning: Applies to **/*.{ts,tsx} : In VeChain Kit Version 2, use `useThor` instead of `useConnex` for contract interactions
Applied to files:
packages/vechain-kit/src/hooks/utils/useAppHubApps.tspackages/vechain-kit/src/utils/constants.tsx
🧬 Code graph analysis (2)
packages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.ts (1)
packages/vechain-kit/src/constants/urls.ts (1)
PRIVY_STATUS_BASE_URL(10-10)
packages/vechain-kit/src/hooks/utils/useAppHubApps.ts (1)
packages/vechain-kit/src/constants/urls.ts (2)
APP_HUB_GITHUB_API_BASE_URL(4-5)APP_HUB_GITHUB_RAW_REPO_BASE_URL(6-7)
⏰ 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). (3)
- GitHub Check: Lint & Build
- GitHub Check: build-size-check
- GitHub Check: deploy
🔇 Additional comments (11)
packages/vechain-kit/src/components/TransactionModal/Components/ShareButtons.tsx (5)
7-7: LGTM: URL constants centralized.Good refactoring to import base URLs from a centralized constants module, improving maintainability and consistency.
35-35: LGTM: Prop rename improves clarity.The rename from
descriptionEncodedtodescriptionbetter reflects that the component now receives raw text and handles encoding internally via URLSearchParams.
38-40: LGTM: Twitter URL construction is correct.The URL construction properly uses Twitter's intent API format with automatic parameter encoding via URLSearchParams.
45-46: LGTM: WhatsApp URL construction looks correct.The WhatsApp share URL properly uses the
textparameter with automatic encoding. The empty path string works correctly when the base URL includes the necessary path component.
51-51: LGTM: Proper URL conversion for href attributes.The use of
.toString()to convert URL objects to strings for thehrefattributes is correct and follows standard practices.Also applies to: 62-62, 69-69
packages/vechain-kit/src/hooks/api/privy/useFetchPrivyStatus.ts (1)
2-2: LGTM! URL centralization implemented correctly.The change to use
PRIVY_STATUS_BASE_URLwith the URL constructor properly centralizes the endpoint configuration and improves maintainability.Also applies to: 6-7
packages/vechain-kit/src/utils/constants.tsx (4)
60-60: LGTM! Image URL centralized.The change to use
IMAGE_NOT_FOUND_URLproperly centralizes the fallback image reference.
116-137: LGTM! Ecosystem app URLs centralized.The migration to base URL constants for all default Privy ecosystem apps improves maintainability and aligns with the PR's centralization objective.
140-154: LGTM! Terms and cookies URLs properly constructed.Both configurations now use the URL constructor with
VECHAIN_KIT_WEBSITE_BASE_URL, ensuring consistent and correct URL formation.
25-48: The path inconsistency is intentional and correct—no issue here.The VOT3 entry uses a leading slash because
COINMARKETCAP_STATIC_BASE_URLlacks a trailing slash, while veDelegate omits the leading slash becauseVECHAIN_TOKEN_REGISTRY_ASSETS_BASE_URLincludes a trailing slash. This design ensures both URLs resolve correctly: VOT3 →https://s2.coinmarketcap.com/17ff70aa1d898bc97ad690dbfad1a3b5643f7e0b.pngand veDelegate →https://vechain.github.io/token-registry/assets/1c641b86096d56bf13d49f38388accd6db8b8b2e.png. Swapping these patterns would break URL resolution.Likely an incorrect or invalid review comment.
packages/vechain-kit/src/hooks/utils/useAppHubApps.ts (1)
4-7: LGTM! GitHub URL construction properly centralized.The refactor correctly uses URL constructors with the centralized base URL constants for both the GitHub API and raw content endpoints. The template literals in the URL paths (e.g.,
master/apps/${dir.name}/manifest.json) work correctly with the URL constructor and maintain the dynamic path generation.Also applies to: 57-58, 69-85, 91-91
This PR centralizes all hardcoded URLs into a single
urls.tsconstants to keep URL usage consistent across the codebase.It also refactors URL construction to use
new URL()(andsearchParamswhere needed) instead of string concatenation/template strings, improving correctness and maintainability.Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.