Tape: Use Streams3 instead of through

[vasiliicuhar]

Tape version: 4.11.0

test.createStream().on('readable', () => console.log('stream has new data'))
test('example', t => t.fail('failed'))

Expected: standard Streams3 behavior (should output 'stream has new data' in console)
Actual: empty console

The problem:
Tape uses through to create output stream in test.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

• not available in browsers (I use tape in https://stackblitz.com environment)
• boilerplate client code

2. return Readable stream from createStream()

• should not break most of the apps, can be done in minor version, we may revert the changes if there will be a lot of affected users

3. deprecate createStream in favor of setOutput(dst: Writable)

• dst can be anything that implements Writable (process.stdout, fs.createWriteStream(), Transform stream, etc)
• flexible lifetime and error management for client code, reasonable defaults (i.e. create write stream on first output chunk)

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, ...)

[ljharb]

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?

[Raynos]

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

function setOutput(test, dst) {
  test.createStream().pipe(dst)
}

I don't see any reason why this one line function should be in tape itself.

[vasiliicuhar]

@ljharb leaving both APIs for a lifetime it is even worse, I would go with @Raynos solution.
@Raynos thanks for rising this concern, indeed with buffered streams auto-piping will be more difficult. But it won't be needed, because buffered streams are "paused" by default and do not loose any data.

What platforms do we really support?

super old versions of both

is not saying anything
Streams3 need to be polyfilled in browsers anyway, and Node supports them since >0.12 (see article)

Let's remember that if we release v5 in some future with new stream api, [email protected] will continue to work in super old versions of both.
I am not pushing it, I just thought that we may explore this area to remove some code from core package, which is in line with keep it minimal approach

[ljharb]

ideally node 0.4 + IE 6; realistically node 0.6 + IE 8.

[vasiliicuhar]

May I ask why this decision to support these old environments was made?
If one doesn't want to upgrade Node.js, he can use older versions of tape.

[Raynos]

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.

[ljharb]

tape also remains the only test runner with this level of support, so it’s pretty important we retain it.