fix(clerk-js): Always invoke an injected Web3 Wallet provider #4734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: e1ae09e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
panteliselef
reviewed
Dec 9, 2024
Comment on lines
56
to
64
| const ethProvider = this.#providers.find(p => p.info.name === this.#providerIdMap[provider])?.provider; | ||
| if (ethProvider !== undefined) { | ||
| return ethProvider; | ||
| } | ||
| return this.#providers.find(p => p.info.name === this.#providerIdMap[provider])?.provider; | ||
|
|
||
| // In case we weren't able to find the requested provider, fallback to the | ||
| // global injected provider instead, if any, to allow the user to continue | ||
| // the flow rather than be a no-op | ||
| return window.ethereum; |
There was a problem hiding this comment.
- Maybe i'm mistaken, but i think we were doing this and we removed it.
- Is there a scenario where if want to request with metamask but the browser does not have it i should throw an error instead ? If yes, maybe the fallback needs to happen at the component level.
There was a problem hiding this comment.
- That's not what we were doing before. We were explicitly trying to find the requested provider (e.g. MetaMask, OKX Wallet etc) and if we couldn't, we weren't initiated the flow, giving the user the impression that the application is broken. If you mean even before the injected web3 provider logic, that was needed in order to avoid multiple web3 wallets to conflict
- Maybe we are looking to throw an error if there isn't a web3 provider available, but we are looking to investigate that in a different effort
panteliselef
approved these changes
Dec 10, 2024
When the requested Web3 provider cannot be found, use any other available injected Web3 Wallet provider, instead of blocking the sign-in/sign-up flow
chanioxaris
force-pushed
the
haris/user-1133-always-invoke-an-injected-web3-wallet
branch
from
78283ea to
e1ae09e
Compare
chanioxaris
deleted the
haris/user-1133-always-invoke-an-injected-web3-wallet
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
If we weren't able to find the exact requested Web3 Wallet provider, then we should fallback and use the injected provider in order for the user to be able to continue the flow
Before
before.mov
After
after.mov
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change