fix: disconnect logic for EIP6963 & Injected provider types for @web3modal/ethers

[hmzakhalid]

Changes

• fix: disconnect logic for EIP6963 & Injected provider types in packages/ethers

Associated Issues

closes #2288

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
web3modal-gallery	✅ Ready (Inspect)	Visit Preview	Jul 10, 2024 4:39pm
web3modal-laboratory	✅ Ready (Inspect)	Visit Preview	Jul 10, 2024 4:39pm

[vercel[bot]]

@hmzakhalid is attempting to deploy a commit to the WalletConnect Team on Vercel.

A member of the Team first needs to authorize it.

[hmzakhalid]

Hey @tomiir, Can you take a look at this Please?

[tomiir]

hey @hmzakhalid ! ty for the contribution. I'm bumping this up with the rest of the team as not sure we want to add more clicks to the connection flow. I'm thinking maybe a proper way to handle this would be with a config flag on the ethers configuration that would dictate if we should or should not revoke the permissions. Agree there are cases where this would be a better ux, but would like input from @glitch-txs @enesozturk as well.

Code looks good overall, would try-catch the requests to the wallet in case they are not supported to prevent unexpected behaviors but otherwise looks good!

[hmzakhalid]

hey @hmzakhalid ! ty for the contribution. I'm bumping this up with the rest of the team as not sure we want to add more clicks to the connection flow. I'm thinking maybe a proper way to handle this would be with a config flag on the ethers configuration that would dictate if we should or should not revoke the permissions. Agree there are cases where this would be a better ux, but would like input from @glitch-txs @enesozturk as well.

Code looks good overall, would try-catch the requests to the wallet in case they are not supported to prevent unexpected behaviors but otherwise looks good!

Thank you for the feedback and for bringing this up with the team. I'd like to point out that this should be the default behavior when the disconnect button is clicked.

• Users expect that clicking a disconnect button will fully disconnect their wallet from the site, including revoking any permissions. This is a standard behavior that users are familiar with, and it aligns with their expectations.
• The wagmi connector, for instance, ensures that the wallet is fully disconnected from the site.
• If the wallet is not fully disconnected, it can lead to confusion and frustration, as users might not understand why they are still connected or why the wallet does not prompt them to reconnect properly. (As it happened in my case).

Would love feedback from the rest of the team.

[tomiir]

Been re-reading this. I'm on board. Verified and it matches wagmi's behavior. Would love to get confirmation from team and we could move ahead with it 💪

[hmzakhalid]

Hey @svenvoskamp , Could you please take a look at this PR as well?

[hmzakhalid]

Shouldn't this disconnect function also be updated? i suppose this is used in the useDisconnect hook https://github.com/hmzakhalid/web3modal/blob/V4/packages/ethers/src/client.ts#L616-L637

@enesozturk @svenvoskamp @tomiir

[enesozturk]

Shouldn't this disconnect function also be updated? i suppose this is used in the useDisconnect hook https://github.com/hmzakhalid/web3modal/blob/V4/packages/ethers/src/client.ts#L616-L637

@enesozturk @svenvoskamp @tomiir

@hmzakhalid That's a good point - yes. The public disconnect() function is for hooks and the JS client and should do the same thing as connectionControllerClient.disconnect does.

[hmzakhalid]

Shouldn't this disconnect function also be updated? i suppose this is used in the useDisconnect hook https://github.com/hmzakhalid/web3modal/blob/V4/packages/ethers/src/client.ts#L616-L637 @enesozturk @svenvoskamp @tomiir

@hmzakhalid That's a good point - yes. The public disconnect() function is for hooks and the JS client and should do the same thing as connectionControllerClient.disconnect does.

Thought so, pushed the updated code.

[tomiir]

Final request: given we are repeating the same thing 3 times, can we abstract it to a method/util? Basically something like disconnectProvider({ provider, providerType }) => void 🙏 Else looks good 🖖 🚀

[hmzakhalid]

Bumping this up @glitch-txs @tomiir