search

ipld / js-block

IPLD Block Interface
6 stars 3 forks source link

feat!: migrate to js-multiformats #15

Closed mikeal closed 4 years ago

rvagg commented 4 years ago

this is all ESM, did you find a way to make it so that it can be consumed by commonjs as well as ESM? I thought that was a hang-up with multiformats that we couldn't get past?

mikeal commented 4 years ago

Here’s the deal.

The only way, today, to support require() is to transpile to CJS and expose a CJS entry point. This would mean using export maps and they aren’t properly supported by compilers yet.

So, for now, it’s ESM only. When the compilers support export maps we can add the transpiling step using rollup (example: https://github.com/MylesBorins/node-osc/blob/current/package.json ).

mikeal commented 4 years ago

This is the way I see it.

I expect to take another relatively small breaking change in the near future once compilers properly support export maps. We have a lot of top level .js files for different export profiles (loaded codecs, hashes and bases) in this module and especially in js-multiformats, and I’d like to move those into lib but there’s no way to make that nice to consume without export maps.

At that time I think it’s worth revisiting what the CJS compatibility story is.

In the meantime, I’m comfortable shipping with ESM only. Even if we supported CJS, you’d need a version of Node.js that has ESM support.

I’d also like to have some time to hear from people how important this is. Supporting this via a compile step isn’t necessary if nobody is asking for it. Given that this is a very new library and people adopting it are more likely than not to be using newer rather than older patterns, this may just be less of an issue that we think it is. I think that some of us, me included, are conditioned to worry a lot about legacy code but there just isn’t a large profile of dependence on this library yet.

rvagg commented 4 years ago

But it becomes viral, like Promises are, in that it forces consumers up the chain to entirely change their code just to consume this library, right? I can't just switch to this library unless I jump in wholesale to ESM; which means I'd probably avoid it if this was only a minor part of a bigger project.

I'm personally not very keen to start converting all my stuff to ESM, the benefits just aren't that great, particularly if you're mainly focused on the server-side use-case. That will probably mean that for some time I'd be avoiding libraries that force that choice on me prematurely, like with Promises.

mikeal commented 4 years ago

I ended up going deep on compiling and exporting dual mode modules. I think I found a way to support it today as long as we map each export entry point to the exact file path we will effectively side-step the lack of compiler support because they will fall back to resolving the esm files by path.

mikeal commented 4 years ago

ok @rvagg, you win 😉

I’ve got require() working now, and we compile all the tests to CJS and run them so we know that the interface is identical.

mikeal commented 4 years ago

I’d like to wait for a review but @rvagg is sick and @vmx is on holiday so I’m going to merge.

rvagg commented 4 years ago

sorry, I'm around, my head is just a bit soupy and slow, +1 on this, I can't review much further without trying to consume it in both ESM and CJS, I guess I'll find out soon!