Improve ESM exports and enable tree shaking

[bpierre]

I identified several areas where we could improve the way Ethers gets exported. Please let me know what you think, and if / how I can help with these 🤗

Marking modules as side-effect free

This might be the easiest improvement: assuming all the Ethers packages are side effect free, marking them as such would make it possible for bundlers to eliminate the imports that are not used.

Note: I tried to do it and run build-all and it seems to impact something in the build configuration: the BN export of bn.js (which is is CJS) doesn’t seem to get converted properly anymore.

Code splitting

The easiest way to eliminate code is to provide exports in ES modules and separate every module in its own file. __PURE__ annotations can help too, but their support varies a lot from a bundler to another. It makes them adapted as an additional optimization, but they don’t really work as a primary way to enable tree shaking.

At the moment, the entire library is bundled in a single file. For the ESM export, setting the output.preserveModules to true and exporting everything into a dist/esm directory would probably be enough.

The package.json browser field

The browser field takes precedence over module and main in some bundlers configured to export for browsers. A consequence of it is that it makes it impossible to use the ESM export of Ethers. Is this field needed at all? Browser support for ES modules is excellent now (with the exception of IE 11 which is close to reach end of life), and most people use a bundler nowadays.

This field could then be useful to provide browser versions of modules (CJS or ESM) that are meant to be used in Node.js environments (see https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced), rather than using it as an alternative to main and module. It could also provide to browsers the same structure than the module export, but minified. It would make import … from "//unpkg.com/ethers" provide an optimized build which could be super nice :)

The ethers named export

Ethers exports the ethers object, which contains all the other named exports. Even though a __PURE__ annotation is being used, it might be difficult for some bundlers to respect it and might prevent any code elimination to happen.

A solution could be to stop exporting this object, and update the documentation to suggest using import * as ethers from 'ethers' instead − while also explaining why individual exports should be preferred. Most bundlers will also accept import ethers from 'ethers' since trying to import a default export from a module not having one is usually converted into import * as … instead.

We could also decide to keep the ethers export. In that case, a solution would be to have it in a separate file (so that apps not using it could tree shake the library), and let users know in the documentation that it is not optimal.

[ricmoo]

Heya!

I would love some help with this. I've spent a lot of time trying to get this as compatible and reliable as possible, but would love someone with more knowledge to take a look.

The browser field is required, especially in the web package (to differentiate between require("http") and fetch) and certain encoding packages such as base64. Everything should be using the browser field as you specified, mocking out a single file, so the exported shape is identical. The web package is probably the best place to see how this is done.

One thing that complicates this is that ethers supports a very wide range of platforms. It supports node 8 and above and most browsers with ES3 (ish) and above. Next summer with v6, these will be greatly relaxed, but for now those are hard requirements. This is also why part of the build process is some complicated and uses myriad scripts in the admin/cmds/ folder. Lerna, TS (composite packages), ESM, rollup, etc. all seem to have slight incompatibilities... But maybe many of those are resolved now, and I should try simplifying the build process again. I will be pulling Lerna out entirely soon as there is only one small portion remaining that I currently use, and should be able to implement over the next few weeks when I get a chance.

I'm not sure if TypeScript has fixed the issues it once had, but at one point I'd found a TS issue where any class with static methods would become un-tree-shakable, which is something I use. If that is fixed, of there is some way to hint to things that it is ok, I'm totally game for adding annotations.

I've tried enabling meaningful tree-shaking before, without much luck.

Everything is side-effect free. The possible exception is the umbrella package, which, in the browser, adds an _ethers global for use in extension packages, something I'd love to eliminate. I try to keep everything as encapsulated as possible. :)

I would like to add a dist/ folder to each package, which would be a good chance to knock out a module value for each packages as well...

Just some thoughts. I've been busy moving and a few other things the last few weeks, I'll look into this more once I get the issue count back under control and have experimented a bit with your annotations you linked to. :)

[wighawag]

This is related to #1011

My current solution is to execute a script to ensure the module field point to content of the browser.esm field but if the browser.esm field is itself an object, the browser field point to the same content : https://github.com/wighawag/purple-warlock/blob/b179a3b7f097bb4eb42036c29ab400dd64588764/fix_ethers_modules.js

This works nicely with https://github.com/vitejs/vite I did not check how much tree shaking it does, but I can at least get code splitting.

Regarding module vs main vs browser,

the current problem is that ethers has non es module on its browser field
both for

1. browser field as object:

   ethers.js/packages/web/package.json

   Line 4 in 86e0269

   "./lib/geturl": "./lib/browser-geturl.js"

2. browser field as string:
   

   ethers.js/packages/random/package.json

   Line 3 in 86e0269

   "browser": "./lib/browser.js",

this break bundlers that expect es module for tree shaking, etc...

Furthermore, modern tooling (like vite gives precedence to es module (the module field is read prior to the browser field) while still taking in consideration the browser field when used an object (to override specific module for browser consumption)

For ethers to work in this case is to

1. use the "module" field for browser es module. This is because node support for es module is not final and once it become, there will probably be a way for library authors to adverstise a specific es module for node consumption
2. make the browser field (as object) to point to es module version of the override

This is currently what I do automatically in my application via the script mentioned above

Do you think the solution I propose would break some use case ?

[wighawag]

By the way, I encountered another issue, this time with a webpack project with the browser field (as object) pointing to non-es-module override version (as oposed to browser es-module override version)

I have a browser-only library that have the module field pointing to es-module (it does not need a browser field as it is browser-only library)

This module has @ethersproject/providers as a peer dependency. The library do not contain @ethersproject/providers as it keep as external and instead expect the application to have it.

When it get imported, it get imported as a es-module. In my particular case the import issue that with a trigger of an import of @ethersproject/providers as es-module, which then trigger an import of @ethersproject/web which then as es-module read the browser field (as object) and then import ../lib.esm/geturl which does not go through the override as the override for that particular path is in the browser.esm field and not in the browser field. This cause the issue that it use the node version of the es-module which then attempt to import "http" which then fails on the browser.

I can fix it by adding the browser.esm override to the browser field too like that :

"browser": {
    "./lib.esm/geturl": "./lib.esm/browser-geturl.js", // was missing
    "./lib/geturl": "./lib/browser-geturl.js"
  },

Note that my script above (that I use in my "vite" project) would also fix that.

The story is more complex though as I could not replicate that in a sample project. Still investigating but though worth mentioning as the flow does make sense and the fact that browser.esm contains information important for es-module loading is not a good sign as this field is non-standard and as you mentioned @ricmoo not expected to be used by application.

[ricmoo]

As an FYI, I have been overhauling the build scripts the last week or so and plan to address this over the next few weeks.

[ricmoo]

This has been largely addressed in 5.0.20. Please try it out.

There are still some issues since there are a large collection of bundlers, each doing things slightly different ways, but I've aimed to make things work as best as I can for the widest possible audience; mainly rollup and Webpack (by extension React Native) should see much better build sizes and better compatibility.

Anyways, try it out and keep the suggestions coming. :)

[csmartinsfct]

@ricmoo I've been playing around with ethers.js in the past few days and during my tests I noticed this line:

import { formatEther } from "ethers/lib/utils";

brings with it a chunk thats 98.3kb big. With webpack v5 that supposedly shouldn't happen - any thoughts on why it is happening? Also the test I did was using Next.js v10 - just so you are aware there is a framework with a specific webpack config that may play a part. Even though in my tests it seems as though named exports are imported correctly with webpack v5 and only the required code is imported.

The code being imported:

./node_modules/@ethersproject/abi/lib.esm/index.js 
./node_modules/@ethersproject/base64/lib.esm/index.js
./node_modules/@ethersproject/hash/lib.esm/index.js
./node_modules/@ethersproject/hash/lib.esm/message.js
./node_modules/@ethersproject/hdnode/lib.esm/index.js 
./node_modules/@ethersproject/json-wallets/lib.esm/_version.js
./node_modules/@ethersproject/json-wallets/lib.esm/index.js 
./node_modules/@ethersproject/json-wallets/lib.esm/keystore.js
./node_modules/@ethersproject/json-wallets/lib.esm/utils.js
./node_modules/@ethersproject/pbkdf2/lib.esm/pbkdf2.js
./node_modules/@ethersproject/random/lib.esm/index.js 
./node_modules/@ethersproject/random/lib.esm/random.js 
./node_modules/@ethersproject/sha2/lib.esm/index.js
./node_modules/@ethersproject/solidity/lib.esm/index.js
./node_modules/@ethersproject/strings/lib.esm/index.js 
./node_modules/@ethersproject/units/lib.esm/index.js 
./node_modules/@ethersproject/wallet/lib.esm/index.js 
./node_modules/aes-js/index.js
./node_modules/ethers/lib/utils.js

Using ethers v5.0.20

Also worth noting it isn't included the whole ethers library so that is something :)

[wighawag]

Using ethers v5.0.20 I can confirm I do not need to use the script mentioned above, it all works well with code splitting (using vite)

[ricmoo]

@csmartinsfct Thanks for the heads up. I will investigate this. :)

[ricmoo]

@wighawag Awesome! Glad to hear it!

[ricmoo]

@csmartinsfct I've investigated a bit, and ethers is not designed to have folders below the root imported from directly. Your code above is actually pulling the non-ESM version in, but I tried pulling in the ESM version directly, and it doesn't do much better (I thought it might handle it, but tree-shaking is still in its infancy).

I may pull the utils out and make it its own sub-modules to help improve these cases, but the better solution for you is probably to add @ethersproject/units to your package.json and install that, then do the import { formatEther } from "@ethersproject/units" directly.

Let me know if that works for you.

[wighawag]

Actually it seems I talked too fast. I guess I had some cached stuff.
I am getting this error with the new es module and no extra script :

Error: Dependency @ethersproject/sha2 is attempting to import Node built-in module crypto.

[wighawag]

it seems that for some reason @ethersproject/[email protected] got installed even though pnpm why @ethersproject/sha2 does not show that version. hmm. I can fix by forcing @ethersproject/sha2 to be ^5.0.6

[ricmoo]

I will lock all versions in the next release to make sure no current package refers to any pre-esm-module version. I am curious why that would happen, but it makes sense to update the package.json files this way anyways.

[csmartinsfct]

@csmartinsfct I've investigated a bit, and ethers is not designed to have folders below the root imported from directly. Your code above is actually pulling the non-ESM version in, but I tried pulling in the ESM version directly, and it doesn't do much better (I thought it might handle it, but tree-shaking is still in its infancy).

I may pull the utils out and make it its own sub-modules to help improve these cases, but the better solution for you is probably to add @ethersproject/units to your package.json and install that, then do the import { formatEther } from "@ethersproject/units" directly.

Let me know if that works for you.

Yea that would work. Thank you :)

[ricmoo]

I think this has been addressed, so I'm going to close it. If not though, please re-open.

Thanks! :)