nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 229 forks source link

What is the point of this module with modern node? #462

Closed kanongil closed 2 years ago

kanongil commented 3 years ago

When I develop modules that expose new types of streams, I have historically extended from the readable-stream classes, rather than the built-in ones. I have done this with the expectation that my modules will have a more stable foundation.

However, I have found that while my foundation is stable, streams extended from readable-stream can cause issues when used from modern node. For example a check for mystream.readableEnded (which is supported in native streams for all active node release lines) always returns undefined, potentially causing all sorts of issues. This means that users will need to take extra care when using my streams, and use outdated and internal ways to access the property.

Given such instability when actually used, I no longer see the point of this module. Is it time to deprecate it? Or at least update the usage instructions with a warning? https://github.com/nodejs/readable-stream/blob/bcbe5e06b10a8915f622e6530f057f7a4ee1a09e/README.md#L57-L61

mcollina commented 3 years ago

I think this module needs updating with the latest changes from Node.js core. It is on my radar but it's quite an endeavour.

MingyaoLiu commented 3 years ago

Should use the web:stream api that supports cross javascript platform. It is included in node 16.5

jimmywarting commented 3 years ago

Should use the web:stream api that supports cross javascript platform. It is included in node 16.5

or just plain iterator/generators

tony-go commented 3 years ago

Could help with a bit of guidance ^^