switch to esm - Githubissues

rvagg / through2

Tiny wrapper around Node streams2 Transform to avoid explicit subclassing noise

MIT License

1.9k stars 106 forks source link

switch to esm #109

Open jimmywarting opened 3 years ago

jimmywarting commented 3 years ago

readable-stream has a few sub dependencies and is lagging behind. We are trying to get rid some of them to reduce the total pkg size. using nodes built in means we will always get the newest/fastest version. So i removed all dependencies and started to require on node:stream instead.
as for the dev dependencies: i updated all of them
got rid of inherits and replaced it with a class/extend instead
test files now needs to call new
i also stop depending on buffer from npm (nodes stream will always upgrade uint8array to buffer if they isn't a Buffer instance)

this calls for a major breaking change

rvagg commented 3 years ago

OK, so I'm not going to say no, but also not yes just yet. I've been doing a dual-publish-like setup with some packages lately where broad compatibility is desirable (pure ESM for some others where I don't care) that's been working quite well. I might use your changes along with that approach to get best of both worlds, but you'll have to give me some time to get around to it.

jimmywarting commented 3 years ago

👍 ok fyi, node-fetch v3 is converting to ESM-only with its 21M downloads / week (pretty big deal) so is all of sindresorhus 1k packages

rvagg commented 3 years ago

Yeah, I know, it's like when we did Promises, we're all getting it whether we like it or not. So yes, I acknowledge that action is reasonable, but this package in particular has very broad and historic usage so hard breakage will be very disruptive and painful to the ecosystem which we can avoid with a bit of care.

mreinstein commented 2 years ago

The title of this PR is slightly misleading since it is both full conversion to ESM and drastically refactoring how the underlying stream plumbing works.

Is there value in doing just the stream plumbing upgrades now, and leaving the more controversial esm conversion as a separate action? Honestly as much as I'm a fan of the esm changes, the stream simplifications in this PR are more valuable cleanup and I'd love to see them sooner. Besides, it's good practice not to lump together large changes in a single PR, dig?