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

fix: delete isMetaMask property and share same EIP-1193 instance with EIP-6963 #1614

Closed
wants to merge 3 commits into from

Conversation

magiziz
Copy link
Contributor

@magiziz magiziz commented Jul 3, 2024

Fixes BX-####
Figma link (if any):

What changed (plus any additional context for devs)

  • window.ethereum.providers causing infinite loop with Rainbow wallet when having Coinbase and MetaMask installed. This is causing a memory leak due to window.ethereum.providers overrides.

EDIT:
This is a small improvement from previous fix we did

Screen recordings / screenshots

What to test

Copy link

github-actions bot commented Jul 3, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-1957759cc6f155ad6d8835015cd8b45eea661fe0
screenshots

@magiziz magiziz marked this pull request as draft July 3, 2024 22:16
@magiziz
Copy link
Contributor Author

magiziz commented Jul 3, 2024

This PR is an alternative to #1613 (review)

@DanielSinclair
Copy link
Collaborator

@magiziz Esteban's PR includes a isMetaMask = false now for the EIP-6963 announcer. Does that accomplish the same thing as this PR?

@magiziz
Copy link
Contributor Author

magiziz commented Jul 4, 2024

@DanielSinclair Yes, that's correct. Instead we delete the isMetaMask property and bring back the walletRouter and window.ethereum.providers. Also the Wagmi team suggested that EIP-6963 and EIP-1193 should share the same instance rather than copying them. This is because, currently, in RainbowKit, you're getting two connections due to them not being the same instance. Here is the code which you can look at https://github.com/wevm/wagmi/blob/main/packages/core/src/actions/reconnect.ts#L71-L75

This is not a major change, but it's a small improvement.

@magiziz magiziz changed the title fix: delete isMetaMask property due to infinite loop fix: delete isMetaMask property and share same EIP-1193 instance with EIP-6963 Jul 4, 2024
@rainbow-me rainbow-me deleted a comment from github-actions bot Jul 4, 2024
Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-7c98fbc6783ecd6a2603a8e0ba180d09d4f45fb4
screenshots

@magiziz magiziz marked this pull request as ready for review July 4, 2024 13:59
Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-efa97308f81c5d0003f125a846f382bf25045659
screenshots

Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-efa97308f81c5d0003f125a846f382bf25045659
screenshots

Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-efa97308f81c5d0003f125a846f382bf25045659
screenshots

Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-26479f3eb2fee93d4d0a68a8a7284f2b28d317ee
screenshots

Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-26479f3eb2fee93d4d0a68a8a7284f2b28d317ee
screenshots

@magiziz magiziz marked this pull request as draft July 4, 2024 17:54
Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-115f5983098172aa4b246882f7698ffb9cc71960
screenshots

@magiziz
Copy link
Contributor Author

magiziz commented Jul 4, 2024

Seems like e2e test fails because we use this e2e app https://bx-e2e-dapp.vercel.app. It requires isMetaMask to be true.

Copy link

github-actions bot commented Jul 4, 2024

Here's the packed extension for this build:
node_modules.tar.gz
rainbowbx-efe5cc8bf4173911008a20e94495be57aae819eb
screenshots

@magiziz magiziz closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants