ESM support for util and rlp
#1948
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Okay, I'm 99.9% confident that the esm build worked without issue. I've tested it locally importing RLP into a vanilla javascript react app and it had no issues (though I'm not 100% sure if that's because the React dev server - not sure if it's rollup or webpack). I also created a really simple HTML document and imported the module directly and was able to call the methods so I'm going to call it a success. Here's the HTML code I used for reference. I just saved this as <!DOCTYPE html>
<html lang="en">
<head>
Welcome to my Module Importer
</head>
<body>
Here's the body
</body>
<script>
import('./dist/index.es.js').then((RLP) => console.log(RLP.encode('hello')))
</script>
</html>Also, just for one further verification step, I logged the RLP object from the script and it produced this WDYT? |
|
If we wanted, we could support ESM for just these two packages for now since they are relatively standalone and I don't think it's a breaking change for people trying to import them. |
|
Following the advice of @ricmoo, I've tested this code in a webworker as well. Here's the html <!DOCTYPE html>
<html lang="en">
<head>
Welcome to my Module Importer
</head>
<body>
Here's the body
</body>
<script>
new Worker('./worker.js')
</script>
</html>Here's the web worker import('./dist/index.es.js').then((RLP) => console.log(RLP.encode('hello')))If you serve this with |
I'm not a fan of libraries doing bundling. I really think that should be something left for the application to do, and each application will use a different bundler and some applications will use no bundlers at all. |
|
I recommend testing against https://github.com/MicahZoltu/preact-typescript-es2015-template if you want to see if your stuff is generally forward compatible and friendly to the future of ES modules on the web. This project uses TS (ultra strict mode) and native modules plus import maps (which is not yet available in all browsers, but a polyfill is included). Hopefully the readme is clear enough for how to add a new dependency, but I would be happy to help walk someone through adding a new dependency to the project (it doesn't use a bundler, so requires a small manual step when a new dependency is added). Feel free to ping me on Discord if someone wants help with that step. |
Sorry, I used the wrong term. I should have said maybe "packaging" since it's just compiling with typescript and then outputting the cjs and esm files. I'm intentionally not bundling any dependencies in our roll-up step and declared them all as external. |
Awesome, will take a look! |
Why use rollup at all? Why not just run |
Basically because @ryanio was having a fair amount of difficulty getting #1936 to work with Since I'm never done ESM builds before, Rollup is specifically designed for building ESM modules (as I understand it), and is a build-tool of choice for many projects, so it seemed reasonable to try using industry standard tools and see how it goes. I'm not opposed to native |
|
Every additional tool in the build chain increases complexity and risk of something breaking, and the more complexity in the build toolchain the harder it is to debug things. TTSC + Transformer plugin is an incredibly thin addition to the toolchain, which helps minimize complexity increase and chance of failure and reduces debugging required when problems are encountered. Rollup on the other hand is a pretty heavyweight tool (though not as bad as webpack) and adds a lot of complexity, and when things break troubleshooting becomes much harder. In #1936 I see the following comment, was there more details somewhere?
|
You'd have to go back and build it locally again. I think there was maybe a place in ethash and then in devp2p where TSC wouldn't compile it with the ESM changes that were made. And karma was having issues as well (though here again, I don't remember the specifics). |
|
Generally speaking, my advise would be to resolve those issues if at all possible. Adding Rollup is likely covering up some underlying problem that may or may not come back to bite you later. |
|
I'm going to close this for now as an experiment worth having done. Will revisit down the road once we decide on how to proceed with regards to ESM support. |
This is a proof of concept introducing the use of rollup to compile our libraries for both ESM and CommonJS, starting with just
rlpandutilsince they have the least number of external dependencies.Steps taken so far:
rlpand `utilpackage-json"module": "[packagename].es.js"key for ESM bundletypeskey per suggestion from @MicahZoltunpm run buildtorollup -cwhich means that Rollup first compiles our code with the Typescript compiler and then outputs CommonJS and ESM bundlests-nodekey to each packagets-configto tell ts-node to usecommonjscompilation or nodejs tests won't runtsconfigto usemodule: "commonjs"in its Typescript compilation step since our code isn't written in ESM format and karma complains otherwiseAdditional thoughts/questions:
utilandrlpare small code bases to begin with).commonjscompiled code versus ESM for the builds? I think the answer is no but I'm not an expert on this one.