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

[bug] Conflict between multiple @walletconnect/ethereum-provider instances when pre-building for cdn #905

Closed
hlopes-ledger opened this issue Feb 23, 2023 · 16 comments
Labels
bug Something isn't working

Comments

@hlopes-ledger
Copy link

hlopes-ledger commented Feb 23, 2023

Link to minimal reproducible example

https://63f7a54331abfc008b46e5ca--ledger-w3o-vite-demo.netlify.app/

Summary

The linked demo is a build of the vite example app from the web3-onboard repo.

Both the Ledger connector (under development) and the WalletConnect connector use the ethereum-provider package. If you connect with one and then try to connect with the other while the connection is active, they will both use the same connection. But if you open the modal for one and close it without connecting, or disconnect before trying to connect with the other, the second one will fail with the error below

Uncaught (in promise) DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "w3m-box-button" has already been used with this registry

... since it will try to create a new component with the same name. Might need to check before creating it.

List of related npm package versions

"@walletconnect/ethereum-provider": "2.4.3"

@hlopes-ledger hlopes-ledger added bug Something isn't working needs review labels Feb 23, 2023
@xzilja
Copy link
Contributor

xzilja commented Feb 24, 2023

Thank you for reporting this, I was able to replicate the issue 👍 will try to isolate concrete error and see if we can fix it on our end, but it would also be a good idea to open similar issue up with web3-onboard team, could be the case where we might not be able to fix this, thus they'll likely need to destroy / recreate instances of ethereum-provider when switching between different connection methods.

@hlopes-ledger
Copy link
Author

hlopes-ledger commented Feb 24, 2023

yes, we're currently updating our web3-onboard Ledger connector for WalletConnect v2 and I'll be mentioning this

@hlopes-ledger
Copy link
Author

We don't even use the modal on our side and set showQrModal: false on EthereumProvider.init, so it might be a matter of not even instantiating it in ethereum-provider in that case.

@xzilja xzilja changed the title Conflict between multiple instances [bug] Conflict between multiple ethereum provider instances [bug] Feb 26, 2023
@muratangin187
Copy link

I am also facing the same issue. In my setup, I have both web3modal and https://github.com/xmtp/xmtp-js I guess the same conflict happens in this case too. Did you find any workaround?

@xzilja xzilja changed the title Conflict between multiple ethereum provider instances [bug] Conflict between multiple @walletconnect/ethereum-provider instances [bug] Mar 9, 2023
@xzilja xzilja changed the title Conflict between multiple @walletconnect/ethereum-provider instances [bug] Conflict between multiple @walletconnect/ethereum-provider instances Mar 9, 2023
@hlopes-ledger
Copy link
Author

We're now implementing WC v2 support in wagmi, and as it loads all the connectors at app startup instead of only when you press a connector button like on web3-onboard, we get an error on the initial page load and only the first instantiated connector will work.

Unhandled Promise Rejection: NotSupportedError: Cannot define multiple custom elements with the same tag name

demo here: https://640a13fb3e6f7d0456b70c50--ledger-next-wagmi-demo.netlify.app

@hlopes-ledger
Copy link
Author

I find it strange that if I only instantiate a Ledger connector that initialises the ethereum-provider with showQrCode: false, and so @web3modal/standalone is not imported, if I do const web3m = window.customElements.get('w3m-box-button') in the console in my app I still get a class instance... not sure where the element is being created if web3modal is not supposed to have been imported

@xzilja
Copy link
Contributor

xzilja commented Mar 15, 2023

Which version of @walletconnect/ethereum-provider is ledger's connector using?

@hlopes-ledger
Copy link
Author

I'm testing with the latest v2 branch, locally linked, so 2.4.10 currently

@xzilja
Copy link
Contributor

xzilja commented Mar 15, 2023

Hmm, that's super weird, it shouldn't even import the package, let alone initialise it as per https://github.com/WalletConnect/walletconnect-monorepo/blob/v2.0/providers/ethereum-provider/src/EthereumProvider.ts#L465-L472

Do you mind sharing your repo / branch for me to investigate if possible? With some instructions on how to run it locally.

@xzilja
Copy link
Contributor

xzilja commented Mar 15, 2023

@hlopes-ledger Just created https://github.com/WalletConnect/walletconnect-monorepo/pull/2105/files

I think this might help with modal becoming a peerDependency, will update this issue once we publish

@hlopes-ledger
Copy link
Author

Thanks, looks like that might solve the issue on our side.
Are you able to build it? Just checked it out but get errors since you still have this import at the top

import type { Web3Modal } from "@web3modal/standalone";

@xzilja
Copy link
Contributor

xzilja commented Mar 15, 2023

You need to add it as a devDependency if you are building from source (not an issue with published dist code, since types are stripped away)

@hlopes-ledger
Copy link
Author

It seems this change does not affect the way we are doing things.

We are serving our Connect Kit library from a CDN and bundling the ethereum-provider libs within, using this method: WalletConnect/walletconnect-monorepo#341 (comment).

From the rollup config on ethereum-provider I see that peerDependencies are being bundled on the UMD file, which we import. I've tried removing peerDependencies on that line but they still get included in the bundle when building ethereum-provider.

Do you mind sharing your repo / branch for me to investigate if possible? With some instructions on how to run it locally.

I'll document this.

@xzilja xzilja changed the title Conflict between multiple @walletconnect/ethereum-provider instances [bug] Conflict between multiple @walletconnect/ethereum-provider instances when pre-building for cdn Mar 17, 2023
@hlopes-ledger
Copy link
Author

hlopes-ledger commented Mar 17, 2023

sorry that this is so involved, but here are the steps to reproduce the issue locally; let me know if something is not clear

  • setup Connect Kit

    git clone https://github.com/LedgerHQ/connect-kit
    cd connect-kit
    git checkout feat/wallet-connect-v2
    cd packages/connect-kit
    yarn && yarn build
    
  • deploy the package somewhere, I use netlify for test builds with netlify deploy -d dist/ --prod

  • go into connect-kit/packages/connect-kit-loader

  • in src/index.ts > loadConnectKit() update src with the deploy URL: https://DEPLOY_ADDRESS/umd/index.js

  • build and link it locally with

    yarn build && yarn link
    
  • setup web3-onboard

    git clone https://github.com/hlopes-ledger/web3-onboard
    cd web3-onboard
    git checkout feat/ledger-wallet-connect-v2
    
  • build and link the Ledger connector using the local Connect Kit loader

    cd packages/ledger
    yarn && yarn link @ledgerhq/connect-kit-loader
    yarn build && yarn link
    
  • go into web3-onboard/examples/with-vite-react, update the walletconnect connector to the latest version and link the local ledger connector

    yarn add @web3-onboard/walletconnect
    yarn
    yarn link @web3-onboard/ledger
    
  • edit src/web3-onboard.ts and make sure both WalletConnect and Ledger are using WalletConnect v2

    const walletConnect = walletConnectModule({
      version: 2,
      projectId: 'YOUR_PROJECT_ID',
    })
    
    const ledger = ledgerModule({
      version: 2,
      projectId: 'YOUR_PROJECT_ID',
      enableDebugLogs: true,
    })
    
  • run the demo app with yarn dev

  • access the demo app in localhost:5173

  • click the "connect" button then "Ledger", close the modal

  • click "Back to wallets" and then "WalletConnect", you'll get Uncaught (in promise) DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "w3m-box-button" has already been used with this registry on the console

  • only the first one of the two connectors will load

I've also local liked ethereum-provider and web3modal, debugged the flow and confirmed that the web3modal import does not happen, so still don't understand why the conflict occurs.

@hlopes-ledger
Copy link
Author

hlopes-ledger commented Apr 3, 2023

An update on this.

Our problem is that we're using the bundled UMD version of ethereum-provider, and in that case rollup converts all dynamic imports into inline ones, so it always bundles and instantiates web3modal. That means that the w3m-box-button component will always be registered even though we don't make use of it, and it will conflict with other instancies that are imported.

We've found a solution on our side, although not the most maintainable one. We copied the EthereumProvider class and the needed types from WalletConnect to our Connect Kit library and removed the references to web3modal. So now we bundle our own and depend on the universal-provider and @walletconnect/utils.

@xzilja
Copy link
Contributor

xzilja commented Apr 6, 2023

I see. We made ample changes around this:

  1. Decoupled and made modal a peerDependency of ethereum-provider
  2. Relaxed hard version pins for the packages within our codebase and partner ones (wagmi)
  3. Set default to not show qrcode modal within ethereum-provider so dynamic import is never hit in nodejs env

I'm not sure if we can do much more here, I think my only suggestion would be moving to esm builds that respect modern features that we utilise, but also understand that this might not be an option in some scenarios.

I'll close this issue, but feel free to comment and @ me if you find some solution that we should review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants