v2 ideas
jonathanong opened this issue · 4 comments
you asked to help maintain this library, so here are some thoughts:
- streams2 implementation, shouldn't be hard with
readable-stream
. - i don't like the type inferencing in
getBody
. prefer if it were like.pipe(cat.string())
,.pipe(cat.buffer())
, etc. by default,.pipe(cat())
should just return an array of all the things. developers should know the type of source stream and how they wish to consume the data. - delegate specific use-cases to other libraries. for example, if you want to concat to a string, just use raw-body (if we get .pipe support). overkill, but i don't feel like reimplementing the stream decoder stuff.
- add
cat(stream, callback)
, basically juststream.pipe(this)
callback(err, data)
- it's a callback, not an event listener, so imo it should haveerr
as the first argument. however,err
should always benull
since concat-stream should never have any errors (i get #6 (comment)) unless we decide to throw errors when there are crazy typing issues. we can do crazy stuff like checklistener.length
but i'm not a fan of that either.- don't re-emit data (#11). it doesn't handle back pressure so it's a terrible idea. people should just pipe to both
cat-stream
and the final stream. yield stream.pipe(cat())
support. not sure how to do that yet - maybe duck type it into a promise.
cb(err, data)
is annoying if there isn't ever an error. Why not just omit that parameter like it presently is?
I'm -1
on the .pipe(cat.string())
and .pipe(cat.buffer())
features but agree that getBody
is too complex.
browserify v3 uses a typed array backed data structure for Buffer now, so you should just always get back a buffer or if the stream has been set to objectMode, you should get an array of objects. That would greatly simplify the getBody
decoding.
Promises and generators would just add clutter to this relatively simple API.
should this library be able to concatenate streams - a bit like what https://github.com/felixge/node-combined-stream ?
if yes, I don't understand "concat-stream should never have any errors" because shouln't concat-stream handle the errors of the underlying streams to report via cb(err, data) ?
What about streams2 implementation??
@jeromew this library is able to concatenate all the data within a single stream into a single datum (string, buffer, etc).
For the record, regarding error-catching ... (since I had to test this to find out)
If you pipe a readable stream to this or any other writable stream (with Readable.protototype.pipe), errors will not be piped downstream along with the data.
When using this library, you must handle upstream errors yourself
function getSomething (cb) {
var readable = getReadableStream();
readable.on('error', end); // <--- handle upstream errors
readable.pipe(concat(function(data){
end(null, data);
}));
var ended = false;
function end (error, data) {
if (ended) return;
ended = true;
cb(error, data);
}
}
Besides any upstream errors, there are no other errors to expect.