maybe use `stream.Readable.toWeb`
jimmywarting opened this issue · 2 comments
stream.Readable.toWeb(streamReadable) got introduced in 17.0.0
while it's not necessary to use it... i would like to know if it brings any benefit from using it over fs.createWriteStream(path)
and instead do stream.Readable.toWeb(fs.createWriteStream(path))
I was just looking at this part of your code, and was considering this question. I came to the conclusion there would be no benefit. You create a Node stream with createWriteStream
but then immediately async iterate over it (using yield *
). Adding the stream.Readable.toWeb
wrapper there would just be overhead that got immediately discarded.
A more aggressive change might be to change BlobDataItem.prototype.stream
from being an async generator to returning a web stream. But since BlobDataItem
is an internal class, I don't see what the benefit in doing that would be, either.
yup, it will just be unnecessary overhead.
the better solution would be to wait for nodejs/node#40606 to be fixed so it would be possible to go direct from file to whatwg stream, the extra overhead of converting it to buffer/node:stream/iterator/uint8array is not worth it...
the best solution would be to get rid of the toIterator entirely so that it never touches the main thread so that we could do something like this instead:
function concatenateReadables(readables) {
const ts = new TransformStream();
let promise = Promise.resolve();
for (const readable of readables) {
promise = promise.then(
() => readable.pipeTo(ts.writable, { preventClose: true }),
reason => {
return Promise.all([
ts.writable.abort(reason),
readable.cancel(reason)
]);
}
);
}
promise.then(() => ts.writable.close(),
reason => ts.writable.abort(reason))
.catch(() => {});
return ts.readable;
}
ref: https://streams.spec.whatwg.org/#example-identity-transform-usages