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.

[wolfy1339]

This issue has popped up again, and it seems the types are not compatible.

Conversion of type 'SonicBoom' to type 'WritableStream' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Property 'writable' is missing in type 'SonicBoom' but required in type 'WritableStream'.ts(2352)

Adding the property that is mentioned in the error leads to another error

Conversion of type 'SonicBoom' to type 'WritableStream' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  The types returned by 'end(...)' are incompatible between these types.
    Type 'void' is not comparable to type 'WritableStream'.ts(2352)

I didn't go further than that

probot/probot#2140

import { Transform } from "node:stream";
import { pino } from "pino";

const t = new Transform();
t.pipe(pino.destination(1));

TypeScript 5.7.2
@types/node v22.12.0
pino v9.6.0