Conversation
WalkthroughThis set of changes introduces a dependency injection (DI) container pattern for service management in the application, refactors balance retrieval logic into a new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ServicesProvider
participant DIContainer
participant WalletBalancesService
participant AuthzHttpService
participant Axios
participant API
App->>ServicesProvider: Initialize
ServicesProvider->>DIContainer: Create container with settings/services
DIContainer->>WalletBalancesService: Instantiate with dependencies
App->>useBalancesQuery: useBalances(address)
useBalancesQuery->>DIContainer: Get walletBalancesService
useBalancesQuery->>WalletBalancesService: getBalances(address)
WalletBalancesService->>Axios: Fetch wallet balances
WalletBalancesService->>AuthzHttpService: Fetch deployment grants
WalletBalancesService->>Axios: Fetch active deployments
WalletBalancesService->>useBalancesQuery: Return aggregated balances
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (28)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (10)
🚧 Files skipped from review as they are similar to previous changes (16)
🧰 Additional context used🧬 Code Graph Analysis (1)apps/deploy-web/src/queries/useBalancesQuery.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (3)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/deploy-web/src/services/balance/wallet-ballances.service.ts (1)
1-1: Fix the typo in the filename.The filename "wallet-ballances.service.ts" contains a typo. It should be "wallet-balances.service.ts" (balances, not ballances).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/providers/ProviderRawData/ProviderRawData.spec.tsx(1 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(1 hunks)apps/deploy-web/src/pages/_app.tsx(3 hunks)apps/deploy-web/src/queries/useBalancesQuery.ts(1 hunks)apps/deploy-web/src/services/balance/wallet-ballances.service.spec.ts(1 hunks)apps/deploy-web/src/services/balance/wallet-ballances.service.ts(1 hunks)apps/deploy-web/src/services/container/createContainer.ts(3 hunks)apps/deploy-web/src/services/decodeInjectedConfig/decodeInjectedConfig.ts(1 hunks)apps/deploy-web/src/utils/apiUtils.ts(2 hunks)apps/deploy-web/tests/seeders/deployment.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: validate-deploy-web
apps/deploy-web/src/queries/useBalancesQuery.ts
[failure] 4-4:
Dependency cycle via "./ServicesProvider:1=>../SettingsProvider/SettingsProviderContext:14=>@src/hooks/useLocalStorage:5=>@src/context/WalletProvider:1=>./WalletProvider:1"
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
[failure] 9-9:
Dependency cycle via "@src/utils/deploymentDetailUtils:10=>./priceUtils:2=>@src/queries/useBlocksQuery:6=>@src/context/SettingsProvider:5=>./SettingsProviderContext:1=>@src/queries:7=>./useApiKeysQuery:5=>@src/context/ServicesProvider:5"
apps/deploy-web/src/services/balance/wallet-ballances.service.ts
[failure] 10-10:
Dependency cycle via "./priceUtils:2=>@src/queries/useBlocksQuery:6=>@src/context/SettingsProvider:5=>./SettingsProviderContext:1=>@src/queries:7=>./useApiKeysQuery:5=>@src/context/ServicesProvider:5=>./ServicesProvider:1"
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test-deploy-web-build
🔇 Additional comments (19)
apps/deploy-web/src/services/decodeInjectedConfig/decodeInjectedConfig.ts (1)
3-5: LGTM! Well-implemented utility function.The
hasInjectedConfigfunction correctly handles:
- SSR compatibility by checking for
windowexistence- Proper truthy evaluation of the injected config property
- Clear return type and descriptive naming
This utility supports the conditional config checking pattern mentioned in the AI summary.
apps/deploy-web/src/components/providers/ProviderRawData/ProviderRawData.spec.tsx (1)
27-44: LGTM! Proper alignment with DI container pattern.The conversion of
axiosandproviderProxymocks from objects to factory functions correctly supports the dependency injection container approach. The mock behavior is preserved while enabling dynamic service instantiation.Key benefits:
- Consistent with the DI pattern introduced in ServicesProvider
- Maintains existing test functionality
- Proper type safety with casting
apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx (1)
92-97: LGTM! Consistent factory function pattern.The conversion of
notificationsApito a factory function maintains the samecreateAPIClientinstantiation while aligning with the dependency injection container pattern. This change preserves test functionality while supporting the new service architecture.apps/deploy-web/src/utils/apiUtils.ts (1)
1-1: LGTM! Improved type consistency.The change from
AxiosInstancetoAxiostype improves consistency with the DI container pattern where Axios instances are passed to service constructors. This is a refinement that maintains functionality while aligning with the broader architectural changes.Also applies to: 131-131
apps/deploy-web/src/services/balance/wallet-ballances.service.spec.ts (3)
17-54: Well-structured test with comprehensive balance verification.The test effectively validates the service's ability to aggregate balance data from multiple sources. The assertions cover all expected balance properties including UAKT/USDC balances, deployment escrows, grants, and active deployments.
57-112: Excellent mock setup with flexible configuration.The setup function provides a clean way to inject custom mock implementations for different service dependencies. The URL-based routing in the Axios mock (lines 64-104) effectively simulates different API endpoints.
114-121: Clean TypeScript interface for test configuration.The
TestInputinterface provides good type safety for the test setup function parameters, making the tests more maintainable and self-documenting.apps/deploy-web/src/pages/_app.tsx (3)
64-68: Improved config loading with proper conditional checks.The addition of
hasInjectedConfig()check prevents unnecessary decoding attempts when no config is injected, improving performance and avoiding potential errors.
71-71: Consistent loading state logic.The loading condition correctly matches the config decoding logic, ensuring the loading indicator only appears when actually needed.
97-113: Well-positioned ServicesProvider in the hierarchy.Moving
ServicesProviderto wrap around other providers makes sense for the DI container pattern, ensuring services are available to all nested components that depend on them.apps/deploy-web/tests/seeders/deployment.ts (1)
7-111: Comprehensive and well-structured test data seeder.The
buildRpcDeploymentfunction provides excellent coverage of the deployment data structure with realistic defaults. The use offakerfor randomization andmergefor overrides makes it flexible for various test scenarios.Key strengths:
- Comprehensive default data covering all deployment properties
- Flexible override system using
DeepPartial<RpcDeployment>- Realistic resource specifications, pricing, and escrow data
- Consistent wallet address and dseq usage throughout the structure
apps/deploy-web/src/services/container/createContainer.ts (4)
1-1: Simplified factory function signatures improve usability.Removing the container parameter from factory functions simplifies the API while maintaining functionality. The direct factory call without parameters (line 17) is cleaner and more intuitive.
Also applies to: 17-17
13-13: Enhanced circular dependency error messaging.Including the current key in the dependency path provides better debugging information when circular dependencies are detected.
30-34: Elegant child container implementation using prototype chaining.The
createChildContainerfunction cleverly uses prototype chaining to inherit from the parent container while adding new factories. This enables the hierarchical DI pattern mentioned in the PR objectives.
36-40: Improved type definitions for better type safety.The updated
DIContainertype usingReturnType<T[K]>and the newFactoriestype alias provide better type inference and constraint checking for the DI container system.apps/deploy-web/src/queries/useBalancesQuery.ts (1)
9-18: Good refactoring to use dependency injection.The hook has been cleanly refactored to use the injected service. The addition of
settings?.apiEndpointto the enabled condition ensures the query doesn't run without proper configuration.apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (3)
16-30: Well-implemented dependency injection pattern.The refactoring to use a typed DI container is a good architectural improvement. The ability to override services via props will make testing much easier.
46-48:Details
❓ Verification inconclusive
Potential initialization issue with service dependencies.
The
walletBalancesServicefactory referencesdi.authzHttpServicefrom the same container being created. This forward reference might cause initialization issues.Consider refactoring to ensure proper initialization order:
- walletBalancesService: () => - new WalletBalancesService(di.authzHttpService, di.axios, browserEnvConfig.NEXT_PUBLIC_MASTER_WALLET_ADDRESS, settings.apiEndpoint), + walletBalancesService: (container) => + new WalletBalancesService(container.authzHttpService, container.axios, browserEnvConfig.NEXT_PUBLIC_MASTER_WALLET_ADDRESS, settings.apiEndpoint),
🏁 Script executed:
#!/bin/bash # Description: Check how createChildContainer handles forward references # Find the createChildContainer implementation echo "=== createChildContainer implementation ===" ast-grep --pattern 'export function createChildContainer($$$) { $$$ }' # Check if other services use similar patterns echo -e "\n=== Other services using di references ===" rg "di\.[a-zA-Z]+Service" --type ts -A 2 -B 2Length of output: 1403
Verify service factory initialization ordering
I wasn’t able to locate how
createChildContainerresolves service factories, so please confirm that forward references (i.e. one factory calling another viadi.otherService) are handled lazily and won’t run into an uninitialized dependency.• File:
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
Lines around:authzHttpService: () => new AuthzHttpService({ baseURL: settings.apiEndpoint }), walletBalancesService: () => new WalletBalancesService( di.authzHttpService, di.axios, browserEnvConfig.NEXT_PUBLIC_MASTER_WALLET_ADDRESS, settings.apiEndpoint ), ...services• If
createChildContainerdoes not defer factory execution until after all services are registered, consider changing the factory signature to accept the container explicitly:- walletBalancesService: () => - new WalletBalancesService(di.authzHttpService, …), + walletBalancesService: (container) => + new WalletBalancesService(container.authzHttpService, …),Please verify the container implementation to ensure no runtime initialization errors.
9-9:Details
❌ Incorrect review comment
Address the dependency cycle issue.
Static analysis indicates a complex dependency cycle involving this import. This needs to be resolved to prevent potential runtime issues.
🏁 Script executed:
#!/bin/bash # Description: Analyze the dependency cycle in detail # Find all files in the cycle path echo "=== Files in the dependency cycle ===" rg -l "deploymentDetailUtils|priceUtils|useBlocksQuery|SettingsProvider|useApiKeysQuery|ServicesProvider" --type ts | sort | uniq # Check if there are any circular imports between services and providers echo -e "\n=== Checking for circular imports between services and context ===" fd --type f --extension ts --exec grep -l "import.*from.*@src/context" {} \; | xargs grep -l "import.*from.*@src/services" | head -10Length of output: 5256
🏁 Script executed:
#!/bin/bash # Verify that no service imports context, to rule out a true circular dependency echo "=== Checking wallet-balances.service.ts for @src/context imports ===" grep -n "import.*from.*@src/context" apps/deploy-web/src/services/balance/wallet-ballances.service.ts || echo "No context imports found in wallet-balances.service.ts" echo -e "\n=== Scanning entire services directory for any @src/context imports ===" grep -R -n "import.*from.*@src/context" apps/deploy-web/src/services || echo "No context imports found in services directory"Length of output: 702
🏁 Script executed:
#!/bin/bash # List all import statements in the wallet-balances service to trace dependencies echo "=== Import lines in wallet-balances.service.ts ===" grep -n '^import ' apps/deploy-web/src/services/balance/wallet-ballances.service.tsLength of output: 738
--- 🏁 Script executed: ```shell #!/bin/bash # Locate and inspect imports in useDenom, deploymentDetailUtils, and apiUtils echo "=== Locate useDenom files ===" fd --type f --extension ts --extension tsx -g "useDenom" echo -e "\n=== Imports in useDenom files ===" for f in $(fd --type f --extension ts --extension tsx -g "useDenom"); do echo "File: $f" grep -n '^import ' "$f" done echo -e "\n=== Imports in deploymentDetailUtils.ts ===" grep -n '^import ' apps/deploy-web/src/utils/deploymentDetailUtils.ts echo -e "\n=== Imports in apiUtils.ts ===" grep -n '^import ' apps/deploy-web/src/utils/apiUtils.tsLength of output: 1085
No circular dependency in WalletBalancesService import
Static analysis flagged a cycle on the import inwallet-balances.service.ts, but:
wallet-balances.service.tsdoes not import any context modules.- The only import cycle lives entirely between
deploymentDetailUtils.ts↔priceUtils.ts, and is already suppressed via// eslint-disable-line import-x/no-cycle.
No runtime or provider–service cycle exists here, so you can ignore this warning.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 GitHub Check: validate-deploy-web
[failure] 9-9:
Dependency cycle via "@src/utils/deploymentDetailUtils:10=>./priceUtils:2=>@src/queries/useBlocksQuery:6=>@src/context/SettingsProvider:5=>./SettingsProviderContext:1=>@src/queries:7=>./useApiKeysQuery:5=>@src/context/ServicesProvider:5"
apps/deploy-web/src/services/balance/wallet-ballances.service.spec.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1358 +/- ##
==========================================
+ Coverage 33.96% 34.24% +0.28%
==========================================
Files 796 799 +3
Lines 19418 19451 +33
Branches 3604 3606 +2
==========================================
+ Hits 6595 6661 +66
+ Misses 12234 12205 -29
+ Partials 589 585 -4
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/deploy-web/src/queries/useBalancesQuery.ts (1)
4-4:⚠️ Potential issueCritical: Resolve the circular dependency instead of suppressing it.
The eslint-disable comment indicates the circular dependency issue from previous reviews remains unresolved. Suppressing the warning doesn't fix the underlying architectural problem that can cause runtime errors and unpredictable initialization order.
Please implement the solution outlined in the previous review: refactor the localStorage logic in
useLocalStorage.tsinto a standalone utility without React hook dependencies, or invert the dependency by having WalletProvider accept settings via props/context instead of importing SettingsProvider directly.
🧹 Nitpick comments (2)
apps/deploy-web/src/services/wallet-balances/wallet-balances.service.ts (2)
36-43: Consider simplifying balance calculation logic.The conditional logic for determining balances could be simplified for better readability:
- const balanceUAKT = - balanceData.balances.some(b => b.denom === UAKT_DENOM) || deploymentGrantsUAKT > 0 - ? parseFloat(balanceData.balances.find(b => b.denom === UAKT_DENOM)?.amount || "0") - : 0; + const uaktBalance = balanceData.balances.find(b => b.denom === UAKT_DENOM); + const balanceUAKT = parseFloat(uaktBalance?.amount || "0");The same pattern applies to USDC balance calculation. The original condition checking for grants doesn't affect the balance calculation logic and might be confusing.
25-25: Consider extracting pagination limit as a constant.The hardcoded value
1000for pagination limit should be extracted as a named constant for better maintainability.+ const MAX_DEPLOYMENTS_PER_REQUEST = 1000; + async getBalances(address: string): Promise<Balances> { // ... - loadWithPagination<RpcDeployment[]>(ApiUrlService.deploymentList(this.apiEndpoint, address, true), "deployments", 1000, this.axios) + loadWithPagination<RpcDeployment[]>(ApiUrlService.deploymentList(this.apiEndpoint, address, true), "deployments", MAX_DEPLOYMENTS_PER_REQUEST, this.axios)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentListRow.tsx(1 hunks)apps/deploy-web/src/components/providers/ProviderRawData/ProviderRawData.spec.tsx(1 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/context/SettingsProvider/settingsStore.ts(1 hunks)apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx(3 hunks)apps/deploy-web/src/hooks/useDeploymentMetrics.tsx(1 hunks)apps/deploy-web/src/hooks/useLocalStorage.ts(1 hunks)apps/deploy-web/src/hooks/useRealTimeLeft.ts(1 hunks)apps/deploy-web/src/pages/_app.tsx(3 hunks)apps/deploy-web/src/queries/useBalancesQuery.ts(1 hunks)apps/deploy-web/src/services/container/createContainer.ts(3 hunks)apps/deploy-web/src/services/decodeInjectedConfig/decodeInjectedConfig.ts(1 hunks)apps/deploy-web/src/services/wallet-balances/wallet-balances.service.spec.ts(1 hunks)apps/deploy-web/src/services/wallet-balances/wallet-balances.service.ts(1 hunks)apps/deploy-web/src/utils/apiUtils.ts(2 hunks)apps/deploy-web/src/utils/priceUtils.ts(0 hunks)apps/deploy-web/tests/seeders/deployment.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/utils/priceUtils.ts
✅ Files skipped from review due to trivial changes (3)
- apps/deploy-web/src/hooks/useDeploymentMetrics.tsx
- apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx
- apps/deploy-web/src/context/SettingsProvider/settingsStore.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/deploy-web/src/services/decodeInjectedConfig/decodeInjectedConfig.ts
- apps/deploy-web/src/pages/_app.tsx
- apps/deploy-web/src/components/alerts/ContactPointCreateContainer/ContactPointFormContainer.spec.tsx
- apps/deploy-web/tests/seeders/deployment.ts
- apps/deploy-web/src/utils/apiUtils.ts
- apps/deploy-web/src/components/providers/ProviderRawData/ProviderRawData.spec.tsx
- apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx
- apps/deploy-web/src/services/container/createContainer.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
apps/deploy-web/src/context/SettingsProvider/settingsStore.ts (1)
settingsIdAtom(5-7)
apps/deploy-web/src/hooks/useLocalStorage.ts (2)
apps/deploy-web/src/context/SettingsProvider/settingsStore.ts (1)
settingsIdAtom(5-7)packages/network-store/src/network.store.ts (1)
selectedNetworkId(70-72)
apps/deploy-web/src/services/wallet-balances/wallet-balances.service.spec.ts (5)
apps/deploy-web/src/services/wallet-balances/wallet-balances.service.ts (1)
WalletBalancesService(12-62)apps/deploy-web/tests/seeders/deployment.ts (1)
buildRpcDeployment(7-111)apps/deploy-web/src/utils/deploymentDetailUtils.ts (1)
deploymentToDto(19-44)apps/deploy-web/src/types/balances.ts (1)
RestApiBalancesResponseType(13-19)packages/http-sdk/src/authz/authz-http.service.ts (1)
AuthzHttpService(56-217)
apps/deploy-web/src/queries/useBalancesQuery.ts (3)
apps/deploy-web/src/types/address.ts (1)
Balances(41-50)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(32-34)apps/deploy-web/src/queries/queryKeys.ts (1)
QueryKeys(1-70)
apps/deploy-web/src/hooks/useRealTimeLeft.ts (1)
apps/deploy-web/src/utils/priceUtils.ts (1)
averageBlockTime(9-9)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test-deploy-web-build
🔇 Additional comments (16)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
27-27: LGTM! Good separation of concerns.Moving the
useRealTimeLefthook to its own dedicated file improves modularity and follows the single responsibility principle. The import change is clean and aligns with the broader refactoring effort.apps/deploy-web/src/hooks/useRealTimeLeft.ts (2)
6-24: Verify block height calculation logic.The hook implementation looks good overall, but there are potential issues with the block height calculations:
Math.abs() usage (lines 13-14): Using absolute value for block differences may not be appropriate. Block heights should be monotonically increasing, so
settledAtandcreatedAtshould typically be less thanlatestBlockHeight. Consider whether negative values indicate an error condition that should be handled differently.Block height assumptions: The calculations assume that
settledAtandcreatedAtrepresent block heights rather than timestamps, which should be verified.Consider replacing the absolute value calculations with proper validation:
- const blocksPassed = Math.abs(settledAt - latestBlockHeight); - const blocksSinceCreation = Math.abs(createdAt - latestBlockHeight); + const blocksPassed = Math.max(0, latestBlockHeight - settledAt); + const blocksSinceCreation = Math.max(0, latestBlockHeight - createdAt);This ensures non-negative values while preserving the expected direction of time.
7-9: Good use of polling interval.The 30-second refetch interval for block data is appropriate for real-time balance calculations, providing a good balance between accuracy and performance.
apps/deploy-web/src/services/wallet-balances/wallet-balances.service.spec.ts (2)
13-55: Excellent test coverage and structure.The test suite demonstrates several best practices:
- Comprehensive mocking: Properly mocks external dependencies (AuthzHttpService, Axios)
- Realistic test data: Uses deployment seeders to generate realistic test scenarios
- Clear assertions: Verifies all returned fields from the
getBalancesmethod- Flexible setup: The setup function allows easy customization for different test cases
The test effectively validates the service's core functionality of aggregating wallet balances, deployment escrow amounts, and deployment grants.
57-112: Well-designed test helper with good mocking strategy.The setup function and mocking approach are well-implemented:
- URL-based routing: The Axios mock intelligently routes requests based on URL patterns
- Flexible input interface: The
TestInputinterface allows precise control over test scenarios- Appropriate fallbacks: Provides sensible defaults when specific inputs aren't provided
The mock implementation correctly simulates the expected API responses for both balance and deployment endpoints.
apps/deploy-web/src/services/wallet-balances/wallet-balances.service.ts (3)
12-18: Excellent dependency injection pattern.The constructor properly implements dependency injection, making the service easily testable and flexible. The readonly modifiers ensure immutability of injected dependencies.
20-26: Good use of parallel execution.Using
Promise.allto execute the three API calls in parallel is an excellent performance optimization. The calls are independent and can safely run concurrently.
51-61: Clean and comprehensive return object.The returned
Balancesobject provides a comprehensive view of wallet state, including all relevant balance types, grants, and active deployments. The structure is well-organized and covers all the necessary data points.apps/deploy-web/src/queries/useBalancesQuery.ts (3)
9-9: Good use of dependency injection pattern.The use of the
useServiceshook to access the DI container is a solid architectural choice that improves testability and separation of concerns.
12-12: Excellent refactoring - delegating to service layer.Moving the balance fetching logic to
walletBalancesServiceimproves modularity and makes the hook a clean interface over the service. This aligns well with the PR objective of leveraging the DI container pattern.
13-13: Good optimization - preventing unnecessary queries.Adding
enabled: !!addressensures the query only runs when an address is available, preventing unnecessary API calls and improving performance.apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (2)
31-31: LGTM: Clean import of the centralized settings atom.The import aligns with the architectural shift towards centralized state management using Jotai atoms.
72-72: LGTM: Proper atom usage for state synchronization.The destructuring to get the setter function follows Jotai best practices for write-only atom access.
apps/deploy-web/src/hooks/useLocalStorage.ts (3)
1-4: LGTM: Imports properly updated for the refactoring.The added imports support the architectural shift to Jotai-based state management and include the necessary performance optimization with
useMemo.
8-8: LGTM: Clean dependency replacement.The replacement of wallet address with
settingsIdAtomsuccessfully decouples the hook from the wallet context while maintaining the same namespacing functionality.
11-24: LGTM: Excellent performance optimization with useMemo.The
useMemowrapper prevents unnecessary recreations of the localStorage functions when other parts of the component re-render, improving performance. The dependency array correctly includes bothselectedNetworkIdandsettingsId.The localStorage key format
${selectedNetworkId}${settingsId ? "/" + settingsId : ""}/${key}maintains the same structure as before, just usingsettingsIdinstead of the wallet address.
2 participants
Why
to have a single container source which can construct services that depend on values provided by React.Providers
What
ServicesProvideruseBalancesto rely on DI container and extractsgetBalancesinto a separate service with service level tests_app.tsxto ensure that<Loading>is not shown if there is no injected config. Config only injected during e2e testing for nowSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores