Skip to content

Conversation

@piglovesyou
Copy link

@piglovesyou piglovesyou commented Feb 3, 2018

Thanks for the great module.

I just wanted to monitor usage of highWaterMark of transforms, which is able to be seen in Node 9.4.0, inside the function "ontransform", and I couldn't. Because currently "parallel-transform" depends on a third party "readable-stream".

Why don't we switch it now?

@Tapppi
Copy link

Tapppi commented Jul 18, 2018

Not the maintainer of this package, but readable-stream is a stable stream base unlike the node core streams package which changes depending on which version of node you are using. It gives someone extending streams a stable base to work on instead of worrying about missing functionality and polyfills etc.

Node 9 and 10 changes will land in readable-stream in the next major release v3, which has a PR open, and should be released relatively soon. I'm guessing @mafintosh will update this package with the new version after it is released.

You should note that 8 is the latest LTS and thus many packages are still catching up to 9 and especially 10 features :)

@Tapppi
Copy link

Tapppi commented Jul 18, 2018

Also, changing to a class makes ParallelTransform() without new not work. So this would be a semver-major change with the only benefit being modernised syntax and slightly faster support for newer additions to the streams module :)

@piglovesyou
Copy link
Author

@Tapppi Sorry for my late reply, I noticed but forgot to make a comment.

Thank you for telling me that. So for the two reasons, it seems we should not erase readable-stream deps from parallel-transform. I'll close this PR, thanks.

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.

2 participants