Update types stream.Readable with NodeJS.ReadableStream#69
Update types stream.Readable with NodeJS.ReadableStream#69dougwilson merged 1 commit intostream-utils:3.0from
Conversation
|
Thank you! I'm not familair with TypeScript, so cannot review this change, but perhaps you can help answer some questions around it that may help me:
Thank you! |
|
/cc @blakeembrey who contributed the original types to take a look too 👍 |
|
SGTM, I think this makes sense. I think To answer 2, it depends if people are up-to-date with |
|
For 1: This would make the library compatible with the many libraries that use the ReadableStream interface instead of the concrete class - https://github.com/DefinitelyTyped/DefinitelyTyped/search?q=readablestream&unscoped_q=readablestream (a bit arbitrary, but it's a bit hard to show that that the interface is used more than the class) For 2: It should be since the interface is "wider" than the class since the class already implements the interface, and I can't imagine any code importing from |
|
Thanks both of you 🤗 . I wanted to add a test if someone can provide number 1 prior to merge, thanks again! |
|
Any update on providing the information above? Also, does this need to have |
In TypeScript, NodeJS.ReadableStream is the interface, whereas stream.Readable is a specific implementation of NodeJS.ReadableStream. I believe this library should work with all readable streams, and so should accept the more general interface rather than just stream.Readable