-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: Unwanted switchChain requests on connect for EVM #5372
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
…efaultNetwork as part of the OptionsController state, rename getDefaultNetwork to getLastActiveOrDefaultNetwork
|
|
@glitch-txs is attempting to deploy a commit to the Reown Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for your contribution! We ask that you please read and sign our CTA Document before we can accept your contribution. You can sign the CTA simply by posting a Pull Request Comment with the following text: I have read the CTA Document and I hereby sign the CTA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
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 fixes unwanted switchChain requests during EVM wallet connection by introducing a defaultNetwork configuration option that allows developers to specify the initial network for connection, preventing unnecessary chain switching.
Key Changes
- Added
defaultNetworktoOptionsControllerstate to store the user's desired initial connection network - Renamed
getDefaultNetworktogetLastActiveOrDefaultNetworkfor clarity about its fallback behavior - Modified Wagmi adapter's
connectmethod to usedefaultNetworkfromOptionsControllerinstead of thechainIdparameter
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/controllers/src/controllers/OptionsController.ts | Added defaultNetwork state property and setDefaultNetwork method with JSDoc documentation |
| packages/controllers/tests/controllers/OptionsController.test.ts | Added comprehensive tests for setDefaultNetwork functionality |
| packages/appkit/src/client/appkit-base-client.ts | Renamed method for clarity and initialized defaultNetwork in OptionsController |
| packages/adapters/wagmi/src/client.ts | Updated connect method to derive chainId from OptionsController.state.defaultNetwork for eip155 chains |
| packages/adapters/wagmi/src/tests/client.test.ts | Added tests verifying chainId behavior with and without defaultNetwork |
💡 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Description
defaultNetworkas part of theOptionsControllerstategetDefaultNetworktogetLastActiveOrDefaultNetworkand added comments.Please include a brief summary of the change.
Type of change
Associated Issues
For Linear issues: Closes APKT-xxx
For GH issues: closes #5365
Showcase (Optional)
If there is a UI change include the screenshots with before and after state.
If new feature is being introduced, include the link to demo recording.
Checklist