Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch to esm #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

switch to esm #109

wants to merge 3 commits into from

Conversation

jimmywarting
Copy link

  • 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
Copy link
Owner

rvagg commented Aug 16, 2021

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
Copy link
Author

👍 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
Copy link
Owner

rvagg commented Aug 16, 2021

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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants