coinbase-wallet-sdk icon indicating copy to clipboard operation
coinbase-wallet-sdk copied to clipboard

Incompatibility issue with Web3Modal in Angular

Open karthedew opened this issue 3 years ago • 14 comments

This is an interesting issue because it works in a React application in scaffold-eth, but not in Angular. The issue originates in declaring ethereum globally as an extension of the Window object. In both WalletLink and Web3Modal, ethereum is extended on the window, but the interfaces declare ethereum differently. When compiling an Angular application, it catches this discrepancy and reports it as an error.

I'm not sure what the right path is. Hoping for some resolution on this without having to change the source code of this package in my project.

WalletLink's global window: https://github.com/coinbase/coinbase-wallet-sdk/blob/master/src/index.ts

declare global {
  interface Window {
    WalletLink: typeof WalletLink
    WalletLinkProvider: typeof WalletLinkProvider
    ethereum?: WalletLinkProvider
    walletLinkExtension?: WalletLinkProvider
  }
}

Web3Modal global window: https://github.com/Web3Modal/web3modal/blob/master/src/components/Modal.tsx

declare global {
  // tslint:disable-next-line
  interface Window {
    ethereum: any;
    BinanceChain: any;
    web3: any;
    celo: any;
    updateWeb3Modal: any;
  }
}

karthedew avatar Feb 17 '22 17:02 karthedew

Thanks for the report, we will investigate a solution.

ewerx avatar Feb 22 '22 01:02 ewerx

Same issue here.

christophercr avatar Mar 09 '22 09:03 christophercr

Same issue here. And I'm using this options in tsconfig.json to allow the crypto usage and avoid common errors related to webpack 5:

  "compilerOptions": {
    "paths":{
      "crypto": ["./node_modules/crypto-browserify"],
      "stream": ["./node_modules/stream-browserify"],
      "assert": ["./node_modules/assert"],
      "http": ["./node_modules/stream-http"],
      "https": ["./node_modules/https-browserify"],
      "url": ["./node_modules/url"],
      "os": ["./node_modules/os-browserify"],
    },

The error output is

 TS2717: Subsequent property declarations must have the same type.  Property 'ethereum' must be of type 'any', but here has type 'CoinbaseWalletProvider'.

10         ethereum?: CoinbaseWalletProvider;
           ~~~~~~~~

  node_modules/web3modal/dist/components/Modal.d.ts:6:9
    6         ethereum: any;
              ~~~~~~~~
    'ethereum' was also declared here.


✖ Failed to compile.
✔ Browser application bundle generation complete.


Error: node_modules/@coinbase/wallet-sdk/dist/index.d.ts:10:9 - error TS2687: All declarations of 'ethereum' must have identical modifiers.

10         ethereum?: CoinbaseWalletProvider;
           ~~~~~~~~

Error: node_modules/@coinbase/wallet-sdk/dist/index.d.ts:10:9 - error TS2717: Subsequent property declarations must have the same type.  Property 'ethereum' must be of type 'any', but here has type 'CoinbaseWalletProvider'.

10         ethereum?: CoinbaseWalletProvider;
           ~~~~~~~~

  node_modules/web3modal/dist/components/Modal.d.ts:6:9
    6         ethereum: any;
              ~~~~~~~~
    'ethereum' was also declared here.

and If we hypothetically change

  node_modules/web3modal/dist/components/Modal.d.ts:6:9
    6         ethereum?: any;
              ~~~~~~~~

the type CoinbaseWalletProvider is different to any, so it will fail

AntonioCardenas avatar Mar 11 '22 08:03 AntonioCardenas

Hello again.

We've just upgraded to the latest version (3.0.6) and I'm afraid the issue is not solved yet. The type for the ethereum property was just changed to unknown which is more restrictive and still not the same as any as it's declared in the Web3Modal lib.

Is there any reason why the ethereum property is set to unknown? If so, then how can we overcome that type mismatch so that we can get our app up and running without enabling TypeScript's skupLibCheck (which is far from ideal)?

christophercr avatar Apr 12 '22 09:04 christophercr

Any news regarding this issue?

christophercr avatar May 04 '22 11:05 christophercr

I haven't tried this compatibility yet, but the window.ethereum was set to unknown. This may have fixed the issue.

I can try this out later this week.

karthedew avatar May 04 '22 11:05 karthedew

In fact setting the property to unknown is a bit restrictive because the developer is "forced" to cast this to the corresponding type according to his needs.

I do understand the reasoning behind this decision however, Web3Modal (one of the most popular libs to support multiple providers in one single place), defines it as any so that no cast is needed which I would say is fair enough considering the amount of providers you can add in there, so that the developer should not add a cast for every single provider.

christophercr avatar May 09 '22 05:05 christophercr

What’s the point of using TS if you’re not going to want to be forced to do that?

ljharb avatar May 09 '22 06:05 ljharb

Well, in fact the issue here is more about integration with other tools in the ecosystem. While I do agree that TS main goal is to type your code, there are some use cases where that cannot cover 100% of your code. This is one of those cases, where there are several libs in the ecosystem and implementing the same restrictions on all of them might not be feasible right now, hence the need to loosen such limitation just by using the any type.

christophercr avatar May 20 '22 06:05 christophercr

Sure - but using unknown means that each developer gets the choice to use any - disabling typechecking - or to cast to the type they expect - retaining the benefit of TS.

If it’s set to any here, nobody has the choice.

ljharb avatar May 20 '22 13:05 ljharb

The other problem you get too is if "ethereum" on the window is optional ethereum?: unknown vs other libs requiring it ethereum: any. Angular does pick this up, but I'm not sure if the casting helps also resolve that issue?

karthedew avatar May 20 '22 14:05 karthedew

I don’t believe that’s requiring it, since the any disabled TS checking anything, but i could be wrong.

ljharb avatar May 20 '22 15:05 ljharb

Well, it depends on the strictness set to the TS compiler in your project. With strictNullChecks enabled the any type doesn't include optional. So yes, it will be a problem for other libs in any project with such configuration which I must say it's pretty much the standard these days.

christophercr avatar May 23 '22 07:05 christophercr

@ljharb in fact, the developer always has the choice to cast to the type he wants, regardless of whether the original type is any, it can be achieved easily by just adding 2 consecutive castings. Right now, he is forced to do so due to the unknown.

But the main issue here is that the project will simply not compile (having the TS compiler option skipLibCheck: false set, which is the default and recommended) due to the type mismatch between the declaration of the same variable in 2 different libs. So I see only two options to solve this problem: 1) set ethereum: any in this lib or 2) set ethereum: unknown in the Web3Modal lib.

But IMHO as I already mentioned before, I would prefer having such type change here rather than in Web3Modal so that no cast is required which I would say is fair enough considering the amount of providers you can add with that modal, so that the developer is not forced to add a cast for every single provider but he'll still have the choice as you mentioned.

christophercr avatar May 23 '22 07:05 christophercr

@bangtoven Let me know if there is anything I can do to help on this.

karthedew avatar Feb 27 '23 19:02 karthedew

hey @karthedew thanks for checking in. @erin-at-work i saw that on your last PR you mentioned whether we would use any instead https://github.com/coinbase/coinbase-wallet-sdk/pull/432/files#r835526498 can you take a look when you get a chance?

bangtoven avatar Feb 27 '23 19:02 bangtoven