Move to TypeScript / TSDX

[bpierre]

Requires Uniswap/web3-react#115

This is an attempt to fix the different issues we have with some connectors that expect a CJS or even Node.js environment.

Changes:

• Move to TypeScript using TSDX.
• Remove the SquareLink connector.
• Add an example using SnowPack (currently broken with WalletConnect / WalletLink).
• Also move to Prettier 2.

[Evalir]

This was such a deep problem 🤯

[Evalir]

🐉 such a rabbit hole...

[sohkai]

Left a couple of small notes, but I'm not entirely sold on esbuild if it introduces this much complexity to the build tooling (and surely, there must be a nice tool out there so we don't have to specify all the flags??).

Is there no other way to support consuming use-wallet with snowpack?

[sohkai]

Oh interesting, but this is assuming no post-bundler, right? Just if you deployed the code snowpack generated by itself?

[bpierre]

I thought it could be nice to start removing not ie > 0 and not dead as they are both included in > 2% now, and to add supports es6-module to be explicit in this SnowPack demo, because we are using native ESM. But it’s not really needed as > 2% already only includes ESM-enable browsers.

[bpierre]

Is there no other way to support consuming use-wallet with snowpack?

No, SnowPack only works with ES modules. It can handle CJS modules through a transform, but it comes with compatibility issues.

Left a couple of small notes, but I'm not entirely sold on esbuild if it introduces this much complexity to the build tooling (and surely, there must be a nice tool out there so we don't have to specify all the flags??).

Having flags rather than a config file is a design decision I think, personally I like it haha. Otherwise, I described the different attempts here: esbuild is the only one getting close, all the other approaches have issues. IMO the main issue with esbuild is that the project is young, but I personally find everything else superior to the others. No config file, plugins, presets, etc.: just one command to run and that’s it.

After this, the plan is to open issues on every library that we are using here, to ask them to support ESM exports, and target browser environments first (rather than Node). A good reason to do so would be to be compatible with tools like SnowPack, Vite or even Webpack 5, which will ship without Node.js polyfills.

Note: an easy solution to these problems would be to only provide CJS exports in use-wallet. That could be a compromise until our dependencies start to export in ESM, but would prevent us to use useWallet() with SnowPack or Vite. We are also discussing about moving useWallet() to TypeScript, which might solve the issue if tsc provides a better support for these modules, especially since some of them are generated by tsc already.

[Evalir]

To add to the discussion:

Left a couple of small notes, but I'm not entirely sold on esbuild if it introduces this much complexity to the build tooling (and surely, there must be a nice tool out there so we don't have to specify all the flags??).

The problem is that, as Pierre says, the approaches used by the other libs, have compatibility issues, and in general it's a hard problem. Pretty much a legacy compatibility mess.

Note: an easy solution to these problems would be to only provide CJS exports in use-wallet. That could be a compromise until our dependencies start to export in ESM, but would prevent us to use useWallet() with SnowPack or Vite.

Indeed, and personally, it's a solution I wouldn't take... It would feel like a step backwards, and we've already committed to using new projects as test beds for these new tools, which have proved to be better overall. I'd push for ESM adoption and just poke everyone about catching up. Webpack 5 is definitely pushing for that direction.

Having flags rather than a config file is a design decision I think, personally I like it haha

I'm definitely the complete opposite on this: I hate not having a config file! But at the same time, as I played with esbuild during the week it has this "no frills" feeling to it, for just doing what it's supposed to do very well and very fast, and that's what I really like. :)

[sohkai]

We are also discussing about moving useWallet() to TypeScript, which might solve the issue if tsc provides a better support for these modules, especially since some of them are generated by tsc already.

This is interesting, and perhaps worth a shot! This would require the end project to use tsc, not just use-wallet, right?

[bpierre]

This is interesting, and perhaps worth a shot!

Yes, trying this now!

This would require the end project to use tsc, not just use-wallet, right?

tsc can export in ESM and CJS, so the end project can use any bundler if we export both (e.g. that’s what Connect does). The only thing would be to ensure that the dependencies get transformed properly, this is what doesn’t work so well at the moment. But since tsc is the compiler that generated the two dependencies we have having issues with (exporting in “CJS-with-__esModule”), I suspect it might solve the issue.

[pedrouid]

Hey guys, if you need me to ship an ESM bundle. Let me know! We could work on this under the WalletConnect monorepo instead of having to hack on top of it

[bpierre]

@pedrouid It would help a lot! Let me know if I can help. I don’t think there is any way to make tsc export in two formats at the same time, so for Connect, we build twice.

[pedrouid]

I've dived into CJS, ESM and UMD earlier this year and it's a world in itself. I found it useful to stick to the simplest setup by having TSC handle the CJS transpilling and then webpack would take that CJS bundle and output a UMD bundle

TSDX is a library that supposedly does all of this out-of-the-box very for all bundles. However there were some caveats and discrepancies so I went with the simpler approach.

What I will attempt instead is to have TSDX build the ESM straight from source and see how it works out. I can follow up in a couple of days with an update

[bpierre]

Sounds good! TSDX is what I am using to convert this project in TS, it seems to export in CJS + ESM by default now.

[pedrouid]

The first issue I hit right away is that TSDX throws me an error for not finding the plugin proposal-class-properties. I wonder if this an issue between Lerna package bootstrapping and TSDX expecting it to be available on the package directory node modules. Any tips? @bpierre

[bpierre]

The first issue I hit right away is that TSDX throws me an error for not finding the plugin proposal-class-properties. I wonder if this an issue between Lerna package bootstrapping and TSDX expecting it to be available on the package directory node modules. Any tips? @bpierre

@pedrouid Do you have a .babelrc declaring it? Otherwise I can have a look at your files, let me know! 🙂

[sohkai]

I was also wondering if it could be a good idea to have a use-wallet/esm-only export.

Yes, I think we should do this, even with the default export (do we really need to have all of those connectors available by default?). It anyway seems like a user concern, e.g. "you can't use provider X with snowpack yet; go ask them to fix it."

I also opened this PR on web3-react, which will allow us to not transform any of the connectors, and let the app bundlers deal with them directly.

Wasn't sure what this means; it wouldn't really change anything right (since snowpack would ultimately be that app bundler anyway? Or could we have it such that the development build only enables X providers but the production build done via another bundler enables all providers?)?

[sohkai]

I wonder if we would be interested in using the names in https://chainid.network/ instead, or if it'd be too verbose?

Do we need to use the names in this library (I would treat this somewhat as a user concern, since it's up to them how they'd like to define the UI per network)

[bpierre]

Do we need to use the names in this library (I would treat this somewhat as a user concern, since it's up to them how they'd like to define the UI per network)

Yes I agree, let’s remove them and networkName 👍 (I’ll do this in a separate PR).

[Evalir]

Also I think it’s still worth exporting useWallet() as ES modules, because bundlers know when to switch to CJS / Node polyfills, even when the entry point is ESM.

I do agree; having an ESM export would be amazing! It's a bummer that Snowpack will need more time, and we should definitely bug library authors to catch on.

[bpierre]

Wasn't sure what this means; it wouldn't really change anything right (since snowpack would ultimately be that app bundler anyway?

With previous versions, we were bundling the external dependencies, with the library e.g. Fortmatic was here: https://unpkg.com/browse/[email protected]/dist/fortmatic-083107e5.js

This makes the connectors more compatible, since all these dependencies are exporting in different ways and converting them was having the effect of making them all provide the same export formats. But an issue with this strategy is that the conversion vary a lot from a connector to another, and we had to disable some of them for this reason.

Another approach is to not bundle them at all, and let app bundlers do this work. I now think it might be a better strategy, so that app authors can decide themselves to discard a connector that doesn’t work well (by not using it / tree shaking it). But a problem preventing this to work is that web3-react is doing const { default: Library } = await import('library'), which only works in certain cases (CJS). And since the dependency is not bundled with use-wallet anymore, it can’t get fixed during the bundling process, so we really need to fix it in web3-react itself.

Or could we have it such that the development build only enables X providers but the production build done via another bundler enables all providers?)?

I’m not sure I understand this, do you mean having a different set of providers based on process.env.NODE_ENV?

[sohkai]

I now think it might be a better strategy, so that app authors can decide themselves to discard a connector that doesn’t work well (by not using it / tree shaking it)

I would agree.

But a problem preventing this to work is that web3-react is doing... so we really need to fix it in web3-react itself.

But regardless, we can't get past the current bundling issue even if we drop most of our default connectors?

I’m not sure I understand this, do you mean having a different set of providers based on process.env.NODE_ENV?

Yes, sort of. I was thinking that during development with a tool like snowpack, you're using ESM modules and don't really need that many connectors anyway (during development, you're likely to only need the injected one unless you're specifically testing the others).

So at a later step, if you use another bundler, e.g. parcel, to build the production site, you can enable the other non-ESM friendly connectors and don't have to worry about it being in ESM.

[bpierre]

But a problem preventing this to work is that web3-react is doing... so we really need to fix it in web3-react itself.

But regardless, we can't get past the current bundling issue even if we drop most of our default connectors?

It would fix the issue yes, but it would mean dropping these connectors: Uniswap/web3-react#115

I just hope this PR could get merged soon in web3-react, so we can keep using some of these connectors in the light (ESM only) version. If if this other PR gets merged, we could also support Magic, which now exports in ESM as well (Fortmatic was only exporting in CJS).

I’m not sure I understand this, do you mean having a different set of providers based on process.env.NODE_ENV?

Yes, sort of. I was thinking that during development with a tool like snowpack, you're using ESM modules and don't really need that many connectors anyway (during development, you're likely to only need the injected one unless you're specifically testing the others).

So at a later step, if you use another bundler, e.g. parcel, to build the production site, you can enable the other non-ESM friendly connectors and don't have to worry about it being in ESM.

I see, I think we should detect the ESM / CJS mode rather than development / production if we do anything like that, but I don’t see how it could be done 🤔

Toggling connectors using process.env.NODE_ENV would be a problem because using ESM or CJS is not necessarily related to being in development or production mode. It is becoming more common nowadays to publish apps as ESM directly (through tools like Snowpack or Rollup to rewrite the import paths at the minimum), now that support for IE11 is disappearing.

Also I think it could be a bit odd to tell users that the connectors available is not only affected by chainId, but also the development mode (i.e. the value of process.env.NODE_ENV). For example, an app could be in development mode only for its UI, while using contracts on the main network, temporarily or because it is a secondary feature. In this scenario, it would be weird to see some connectors only working in production mode.

I see two approaches at the moment (both requiring Uniswap/web3-react#115):

• Use a separate module, only containing ESM-enabled connectors (this could be published in a directory, e.g. use-wallet/esm).
• Use dynamic import()s for app connectors. It would completely eliminate the issue because connectors would be loaded as needed. If a connector is incompatible, it wouldn’t be visible until the connection is initiated. We would have to ensure the resolution of the module format (CJS vs. ESM) for the connectors is properly done by bundlers.

I think the latter one would be ideal if it works as expected!

[bpierre]

I merged the PR: the ESM / CJS issues are fixed, the only problem now is that several dependencies are relying on Node.js polyfills, making it difficult to use in SnowPack: this will be addressed separately.