Question: What's the best practice for calling callback() in stream.Writable._write()? #5234

Closed
hueniverse opened this Issue Apr 7, 2013 · 3 comments

Comments

Projects
None yet
2 participants

I've got a writable stream I'm using to make it easy for multiple consumers to peek into a stream being piped. For multiple reasons, they can't just join the pipe party. But this doesn't matter for the purpose of the question.

The _write() method just emits the chunk to those listening. Once done, it calls callback():

MyStream.prototype._write = function (chunk, encoding, callback) {

    this.emit('peek', chunk, encoding);
    callback();
};

But as I'm nextTickifying all our interfaces to provide a consistent API handling of callback, I wonder if I should change this to:

MyStream.prototype._write = function (chunk, encoding, callback) {

    this.emit('peek', chunk, encoding);
    process.nextTick(function () {
        callback();
    });
};

What's the BCP?

isaacs commented Apr 7, 2013

The Writable.write function actually expects that people will occasionally do it wrong, and detects if you have called the cb in this tick or not.

So this is perfectly fine:

util.inherits(Peeker, Writable);
function Peeker(opt) {
  if (!(this instanceof Peeker))
    return new Peeker(opt);
  Writable.call(this, opt);
}
Peeker.prototype._write = function(chunk, enc, cb) {
  this.emit('peek', chunk, enc);
  cb();
};

isaacs closed this Apr 7, 2013

You are still calling it 'doing it wrong'... :) even if you are prepared to handle it. How would you write this? with or without explicit nextTick?

isaacs commented Apr 7, 2013

I would just go ahead and call it right away. We do this with somewhat reckless abandon in some internal streams, it's perfectly fine. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment