-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
Use Streams3 instead of through #478
Comments
Regarding solution options, it must work (or have a super graceful degradation) in both node and browsers, going back to super old versions of both. Adding an alternative to createStream that achieves what you want (without deprecating createStream, necessarily) seems fine, if it can meet the above constraint. Thoughts? |
This part of the code can be quite fragile since there's a lot of logic around "auto piping" to stdout if and only if no one is reading from the main default tape stream. You can always use your own implementation of setOutput
I don't see any reason why this one line function should be in tape itself. |
@ljharb leaving both APIs for a lifetime it is even worse, I would go with @Raynos solution. What platforms do we really support?
is not saying anything Let's remember that if we release v5 in some future with new stream api, tape@4.x will continue to work in super old versions of both. |
ideally node 0.4 + IE 6; realistically node 0.6 + IE 8. |
May I ask why this decision to support these old environments was made? |
The choice is mostly for library authors. If you author a library instead of an application and you want the library to work on IE6+ and all versions of node then you need to use a testing library to run your tests on all those environments. As an application author you just test against node10 and can use a testing library that supports node10 and higher. |
tape also remains the only test runner with this level of support, so it’s pretty important we retain it. |
Tape version: 4.11.0
Expected: standard Streams3 behavior (should output 'stream has new data' in console)
Actual: empty console
The problem:
Tape uses
through
to create output stream intest.createStream()
. This module has aged significantly and Streams3 were introduced since then (see https://nodejs.org/api/stream.html#stream_compatibility_with_older_node_js_versions for detailed description).Solutions:
1. use stream.wrap on client side
2. return Readable stream from createStream()
3. deprecate createStream in favor of
setOutput(dst: Writable)
I can start working on PR for setOutput()/ createStream() next week.
What do you think?
#338
We can see if we can handle
dst
stream close event instead of relying on process.on('exit, ...)The text was updated successfully, but these errors were encountered: