This repository has been archived by the owner. It is now read-only.

Readable stream : objectMode ignored? #4662

Closed
naholyr opened this Issue Jan 25, 2013 · 7 comments

Comments

Projects
None yet
4 participants
@naholyr

naholyr commented Jan 25, 2013

Code to be tested:

// array → Readable stream
function ArrayReadStream (array, options) {
  stream.Readable.call(this, options);
  this._index = 0;
  this._array = array;
}
ArrayReadStream.prototype = {
  __proto__: stream.Readable.prototype,
  _read: function (n, cb) {
    var self = this;
    process.nextTick(function () {
      cb(null, (self._index < self._array.length) ? self._array[self._index++] : null);
    });
  }
};

// Writable stream → console.log
function DebugStream (options) {
  stream.Writable.call(this, options);
}
DebugStream.prototype = {
  __proto__: stream.Writable.prototype,
  _write: function (data, cb) {
    console.log(String(data), JSON.stringify(data));
    cb();
  }
};

// Test
var array = [
  {value: "x"},
  {value: "y"}
];

var options = {
  objectMode: true
};

new ArrayReadStream(array, options).pipe(new DebugStream());

As expected, it outputs:

[object Object] {"value":"x"}
[object Object] {"value":"y"}

But what I'm not expecting, is the same exact output with objectMode: false.

I was expecting a fallback to previous behavior, which still was buggy in my opinon, but still less that ignoring an option ;)

Is this expected? What am I missing?

Note: in 0.9.7 this code just doesn't write anything, it just stops reading stream as soon as a non-string-or-buffer value is met, without emitting "end" ever.

@naholyr

This comment has been minimized.

Show comment
Hide comment
@naholyr

naholyr Jan 25, 2013

Seems to be a minsunderstanding about the role of objectMode option, which is not about objects but about ignoring "n" value in _read.

Maybe an issue in my brain, or in documentation, not sure yet.

naholyr commented Jan 25, 2013

Seems to be a minsunderstanding about the role of objectMode option, which is not about objects but about ignoring "n" value in _read.

Maybe an issue in my brain, or in documentation, not sure yet.

@isaacs

This comment has been minimized.

Show comment
Hide comment
@isaacs

isaacs Jan 25, 2013

The reason why it does the same thing with objectMode: false is that both the Readable and Writable classes automatically flip into objectMode when you give them something other than a string or buffer. So, this is working as designed, but as you point out, the design is perhaps a bit too magical.

Maybe what we need is a objectMode: never ever srsly, I mean it option?

isaacs commented Jan 25, 2013

The reason why it does the same thing with objectMode: false is that both the Readable and Writable classes automatically flip into objectMode when you give them something other than a string or buffer. So, this is working as designed, but as you point out, the design is perhaps a bit too magical.

Maybe what we need is a objectMode: never ever srsly, I mean it option?

@isaacs isaacs closed this Jan 25, 2013

@naholyr

This comment has been minimized.

Show comment
Hide comment
@naholyr

naholyr Jan 25, 2013

Hmm, maybe false (when explicitly set) should be enough :P

naholyr commented Jan 25, 2013

Hmm, maybe false (when explicitly set) should be enough :P

@isaacs

This comment has been minimized.

Show comment
Hide comment
@isaacs

isaacs Jan 30, 2013

Sure. Explicit false should prevent implicit objectMode.

isaacs commented Jan 30, 2013

Sure. Explicit false should prevent implicit objectMode.

@isaacs isaacs reopened this Jan 30, 2013

@ghost ghost assigned isaacs Jan 30, 2013

@Raynos

This comment has been minimized.

Show comment
Hide comment
@Raynos

Raynos Feb 21, 2013

@isaacs I also don't mind objectMode defaulting to false and no magical conversion and it just converts it into a Buffer or crashes or some bullshit.

I'm going to always manually pass in { objectMode: true } for my streams

Raynos commented Feb 21, 2013

@isaacs I also don't mind objectMode defaulting to false and no magical conversion and it just converts it into a Buffer or crashes or some bullshit.

I'm going to always manually pass in { objectMode: true } for my streams

@TooTallNate

This comment has been minimized.

Show comment
Hide comment
@TooTallNate

TooTallNate Feb 21, 2013

+1 for what @Raynos said

TooTallNate commented Feb 21, 2013

+1 for what @Raynos said

@isaacs

This comment has been minimized.

Show comment
Hide comment
@isaacs

isaacs Feb 23, 2013

@Raynos @TooTallNate See #4835

Let's just remove the objectMode automatic switch entirely. It's too magical.

isaacs commented Feb 23, 2013

@Raynos @TooTallNate See #4835

Let's just remove the objectMode automatic switch entirely. It's too magical.

isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Feb 23, 2013

stream: Do not switch to objectMode implicitly
Only handle objects if explicitly told to do so in the options
object.  Non-buffer/string chunks are an error if not already in
objectMode.

Close #4662

@isaacs isaacs closed this in 3404608 Feb 25, 2013

isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Mar 3, 2013

stream: Do not switch to objectMode implicitly
Only handle objects if explicitly told to do so in the options
object.  Non-buffer/string chunks are an error if not already in
objectMode.

Close #4662
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.