-
Notifications
You must be signed in to change notification settings - Fork 1
feat: improve our automatic network switching logic #140
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a parameterless, app-level Changes
Sequence Diagram(s)sequenceDiagram
participant App as AppContent
participant Hook as useAutoNetworkSwitch
participant State as AppKit State (accounts/networks)
participant Modal as AppKit Modal (switchNetwork)
App ->> Hook: invoke useAutoNetworkSwitch()
Hook ->> State: read currentNamespace, evmConnected, solanaConnected, networks
alt target network resolved
Hook ->> Modal: modal?.switchNetwork(targetNetwork)
Modal -->> Hook: switch result (success/failure)
else no target
Hook -->> App: no action
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts (1)
10-14: Consider edge case: empty or malformed caipNetworkId.If
caipNetworkIdis an empty string or malformed,currentNamespacecould be an empty string or unexpected value. While unlikely in normal operation, adding a guard or fallback could improve robustness.Example defensive check:
- const currentNamespace = caipNetwork?.caipNetworkId?.split(':')[0] + const currentNamespace = caipNetwork?.caipNetworkId?.split(':')[0] || undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/agentic-chat/src/app/app.tsx(2 hunks)apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx(2 hunks)apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/agentic-chat/src/app/app.tsx (1)
apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts (1)
useAutoNetworkSwitch(5-42)
⏰ 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: main
🔇 Additional comments (6)
apps/agentic-chat/src/app/app.tsx (1)
7-7: LGTM! Clean integration of automatic network switching.The hook is appropriately placed at the app level in
AppContent, ensuring automatic network switching behavior is active throughout the application lifecycle.Also applies to: 48-48
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (3)
1-1: LGTM! Cleaned up unused imports.The removal of
mainnet,solana, andmodalimports aligns with delegating network switching to theuseAutoNetworkSwitchhook.
70-73: LGTM! Consistent with EVM disconnect approach.The Solana disconnect handler follows the same pattern as the EVM handler, appropriately delegating network switching to the automatic hook.
65-68: The simplified disconnect logic withuseAutoNetworkSwitchis well-designed.The hook correctly includes
evmConnectedandsolanaConnectedin its dependency array, ensuring it reliably reacts to account state changes after async disconnect completes. ThehasAttemptedSwitchRefprevents race conditions and duplicate switch attempts, with error handling that resets the flag if a switch fails. Whendisconnect({ namespace: 'eip155' })completes and updates the account state, the hook's effect automatically re-runs and switches to Solana if needed.apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts (2)
2-2: LGTM! Self-contained hook design improves encapsulation.The refactored hook derives all necessary state internally from AppKit hooks, eliminating the need for external parameters and making it easier to use throughout the application.
Also applies to: 5-8
38-41: LGTM! Reset mechanism prevents stuck state.The separate effect that resets the attempt flag when the network changes ensures the hook can respond to future network changes appropriately, preventing the flag from permanently blocking legitimate switches.
cd6a952 to
9da7495
Compare
9da7495 to
5f0ffdc
Compare
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)
apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts (1)
6-15: Redundant'evm'namespace check remains unaddressed.Line 15 still checks for both
'eip155'and'evm'namespaces. As noted in a previous review, AppKit v1.8.2 only returns'eip155'for EVM chains (CAIP-2 format). The'evm'check will never match.- const isEvmNamespace = currentNamespace === 'eip155' || currentNamespace === 'evm' + const isEvmNamespace = currentNamespace === 'eip155'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/agentic-chat/src/app/app.tsx(2 hunks)apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx(2 hunks)apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/agentic-chat/src/app/app.tsx
- apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx
⏰ 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: main
🔇 Additional comments (1)
apps/agentic-chat/src/hooks/useAutoNetworkSwitch.ts (1)
1-4: LGTM on imports.Clean imports with proper type annotation for
AppKitNetwork.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.