Offer a SonicBoomWritableStream class that can be pipelined with other streams

[mattbishop]

I want to use SonicBoom with nodes' pipe() and pipeline() functions, but as an EventEmitter, it cannot play directly. A SonicBoomWritableStream would have the extra methods to make it a pipeline able data sink.

Perhaps an easy node wrapper exists that I'm not aware of?

[mcollina]

what errors are you getting? can you add a simple snippet to repro?

[mattbishop]

import {pipeline} from "stream/promises"

const sonicBoom = new SonicBoom({dest:`${outDir}/base_amounts.xml`})
await pipeline(aReadStream, transform1, transform2, sonicBoom);

TSError: ⨯ Unable to compile TypeScript:
src/index.ts:14:9 - error TS2769: No overload matches this call.
  Overload 1 of 7, '(source: Readable, transform1: Transform, destination: Writable, options?: PipelineOptions | undefined): Promise<void>', gave the following error.
    Argument of type 'EventEmitter' is not assignable to parameter of type 'PipelineOptions'.
      Property 'signal' is missing in type 'EventEmitter' but required in type 'PipelineOptions'.
  Overload 2 of 7, '(source: Readable, transform1: Transform, transform2: PipelineTransform<Transform, any>, destination: WritableStream | PipelineDestinationIterableFunction<...> | PipelineDestinationPromiseFunction<...> | PipelineDestinationIterableFunction<...> | PipelineDestinationPromiseFunction<...>, options?: PipelineOptions | undefined): Promise<...> | Promise<...>', gave the following error.
    Argument of type 'Writable' is not assignable to parameter of type 'PipelineTransform<Transform, any>'.
      Type 'Writable' is missing the following properties from type 'ReadWriteStream': readable, read, setEncoding, pause, and 6 more.
  Overload 3 of 7, '(stream1: ReadableStream, stream2: WritableStream | ReadWriteStream, ...streams: (PipelineOptions | WritableStream | ReadWriteStream)[]): Promise<...>', gave the following error.
    Argument of type 'EventEmitter' is not assignable to parameter of type 'PipelineOptions | WritableStream | ReadWriteStream'.
      Type 'EventEmitter' is missing the following properties from type 'ReadWriteStream': readable, read, setEncoding, pause, and 10 more.

It's a Typescript error, but the node docs for pipeline() don't indicate one can use an EventEmitter as a destination. https://nodejs.org/api/stream.html#streampipelinestreams-callback

[mcollina]

My understanding is that it should work

[mcollina]

it's just a TS issue, would you like to send a PR?

[mattbishop]

This works on Node 16, so it might be a @types/node issue:

const { pipeline, Readable } = require("node:stream")
const SonicBoom = require("sonic-boom")

// works
Readable.from("Hello pipe()!")
  .pipe(new SonicBoom({dest: "./pipe.txt"}))

// works
pipeline(
  Readable.from("Hello pipeline()!"),
  new SonicBoom({dest: "./pipeline.txt"}),
  () => console.log("done"))

It might not be a typing issue however, as the pipe/pipeline code calls utils.isWritableNodeStream() and a SonicBoom instance looks like one:

https://github.com/nodejs/node/blob/3bc23f555a8501f5bb37a78cbe866102d02984b5/lib/internal/streams/utils.js#L28

So I'm not sure how to file an issue with @types/node to fix this, since EventEmitter alone is not sufficient to participate in the pipeline process. I'll have to force TS to accept SonicBoom to be a WritableStream with as WriteableStream or similar.

[mcollina]

I concur with your analysis. What's missing in @types/node is an Interface for these kind of modules. This is an expected use case for .pipe() and pipeline. However this not a problem for this module, so maybe open an issue on DefinitelyTyped?

[mcollina]

Would you like to send a PR to document this in the README? So at least other people will know they have to cast.

[mattbishop]

I think I'll try creating a SonicBoomWritableStream first and see how it looks. I don't think I will make much progress on the node front!

[mattbishop]

It looks less-disruptive to expand SonicBoom and add the WritableStream methods instead of making a separate class. Any objections?

[mcollina]

I don't think there is a need for any code change, a cast is totally ok and not bad.