Compat w/ Brave to support multi web3 providers #7728
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,5 @@ | ||||||
| /*global chrome*/ | ||||||
|
|
||||||
| import pump from 'pump' | ||||||
| import querystring from 'querystring' | ||||||
| import LocalMessageDuplexStream from 'post-message-stream' | ||||||
|
|
@@ -21,8 +23,19 @@ const inpageBundle = inpageContent + inpageSuffix | |||||
| // MetaMask will be much faster loading and performant on Firefox. | ||||||
|
|
||||||
| if (shouldInjectProvider()) { | ||||||
| // If this is Brave, it requires coordination to know if we should be the | ||||||
| // web3 provider. | ||||||
| if (chrome.braveWallet && chrome.braveWallet.getWeb3Provider) { | ||||||
|
There was a problem hiding this comment.
Suggested change
Correct me if I'm wrong, but I suspect that the There was a problem hiding this comment. Yeah, since |
||||||
| chrome.braveWallet.getWeb3Provider((provider) => { | ||||||
| if (provider === extension.runtime.id) { | ||||||
| injectScript(inpageBundle) | ||||||
|
There was a problem hiding this comment. I agree with @Gudahtt that injecting async is asking for trouble, so I really recommend addressing that for Brave's sake. Since this doesn't introduce any async operation to our WebExtension-based injection, I'm okay with this. There was a problem hiding this comment. Yep I'll get it working sync. There was a problem hiding this comment. I'll add the typeof check on chrome too. |
||||||
| start() | ||||||
| } | ||||||
| }) | ||||||
| } else { | ||||||
| injectScript(inpageBundle) | ||||||
| start() | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
|
|
||||||
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.
I was going to suggest that you use
extensionizerinstead, but that wouldn't work - that doesn't expose thebraveWalletAPI.Maybe we should update
extensionizerto add that API? 🤔 In the meantime, this seems OKThere 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.
If
braveWalletis a similar API to thechrome/extensionAPIs that are common in browsers, then it would be a good fit. By the name, it sounds more like a provider..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.
braveWalletis an additional API. It would need to be added to the list of APIs inextensionizer. At the moment, that package only exports the APIs in that list.It's not a cross-browser API of course, so I wasn't sure whether it was a good fit for that package.