Skip to content
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

feat: improves the react connected hook when using extension & emit terminate when using extension #1186

Merged

Conversation

christopherferreira9
Copy link
Collaborator

@christopherferreira9 christopherferreira9 commented Jan 8, 2025

Explanation

This PR aims to improve the react connected hook when using the extension. Previously, the connected hook would always be true when using extension because window.ethereum.isConnected is always true as long as the extension is being detected. This now verifies if we have an account present meaning that it is actually connected to extension. If no account is present then it assumes it has been disconnected like mobile does (even though mobile relies on socket events to do this).

Main changes:

  • terminate.ts. now emits a terminate event when extensionOnly is true (the current default)
  • adds a new useHandleTerminateEvent.ts hook to handle the behavior of some connected hooks such as connecting and connected
  • MetaMaskProvider.tsx now sets connected based on the existence of an account or not since window.ethereum.isConnected is always true if extension is installed

Before

Screen.Recording.2025-01-08.at.4.15.33.PM.mov

After

Screen.Recording.2025-01-08.at.4.14.26.PM.mov

The Terminate event can also be caught from the dapp side when using extension:

Screenshot 2025-01-08 at 4 18 22 PM

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@christopherferreira9 christopherferreira9 requested a review from a team as a code owner January 8, 2025 12:39
@christopherferreira9 christopherferreira9 marked this pull request as draft January 8, 2025 16:19
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.58%. Comparing base (4151089) to head (7557bd0).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
packages/sdk-react/src/MetaMaskProvider.tsx 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1186      +/-   ##
==========================================
+ Coverage   74.40%   74.58%   +0.18%     
==========================================
  Files         181      182       +1     
  Lines        4306     4321      +15     
  Branches     1057     1059       +2     
==========================================
+ Hits         3204     3223      +19     
+ Misses       1102     1098       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christopherferreira9 christopherferreira9 changed the title chore: improves the react connected hook when using extension chore: improves the react connected hook when using extension & emit terminate when using extension Jan 8, 2025
@christopherferreira9 christopherferreira9 marked this pull request as ready for review January 8, 2025 16:56
@christopherferreira9 christopherferreira9 added the deploy Deploys the test dapps label Jan 8, 2025
@christopherferreira9 christopherferreira9 marked this pull request as draft January 8, 2025 17:18
@abretonc7s abretonc7s changed the title chore: improves the react connected hook when using extension & emit terminate when using extension feat: improves the react connected hook when using extension & emit terminate when using extension Jan 9, 2025
@christopherferreira9 christopherferreira9 marked this pull request as ready for review January 9, 2025 13:01
@christopherferreira9 christopherferreira9 merged commit 801c9ef into main Jan 13, 2025
36 checks passed
@christopherferreira9 christopherferreira9 deleted the chore/improve-react-connected-hook-behavior branch January 13, 2025 13:13
@christopherferreira9 christopherferreira9 mentioned this pull request Jan 14, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploys the test dapps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants