readable.unshift(chunk) not resetting state.reading

[zoubin]

I see it is done on purpose in readableAddChunk, but it confuses me. Especially when I find that the example given in the documentation fails because it does nothing to reset the reading flag (like push('')) after this.unshift(b).

Is it necessary to behave this way?

[jasnell]

@trevnorris

[trevnorris]

Sorry. Don't fully understand the concern.

May also be advantageous to pull in @chrisdickinson

[jasnell]

@trevnorris @chrisdickinson ... if I understand the original post correctly, the question is about why readable_stream.unshift does not cause readablestate.reading to be reset the way readable_stream.push does. See https://github.com/joyent/node/blob/master/lib/_stream_readable.js#L128 vs. https://github.com/joyent/node/blob/master/lib/_stream_readable.js#L134. The final argument to readableAddChunk is addToFront, the reading flag is only reset if addToFront is false. The original poster is looking to understand why that is.

[trevnorris]

Ah. Um, no idea. I think @isaacs was the one to implement that code.

[jasnell]

@isaacs ... any chance you could help shed some light on this one?

[zoubin]

@jasnell @trevnorris Thanks for response. Here is a more simple example to show my confusion:

var rs = require('stream').Readable({ objectMode: true })
var buffer = ['a', 'b']
var unshift = false

rs._read = function () {
  if (!unshift) {
    this.unshift('c') // this leaves rs._readableState.reading `true`
    unshift = true
    this.push('') // this makes rs._readableState.reading `false`
  }
  else {
    if (buffer.length) {
      this.push(buffer.pop())
    }
    else {
      this.push(null)
    }
  }
}

console.log('data:"' + rs.read() + '"')
console.log(rs._readableState.reading)
console.log('data:"' + rs.read() + '"')
console.log('data:"' + rs.read() + '"')
console.log('data:"' + rs.read() + '"')

output:

data:"c"
false
data:""
data:"b"
data:"a"

However, if we do not .push('') after .unshift('c'), rs._readableState.reading will be leaved true and the following three reads get nothing

output:

data:"c"
true
data:"null"
data:"null"
data:"null"

I believe that .unshift() not resetting state.reading makes .unshift() have to be followed by a .push() to make things work. Is it necessary?

[zoubin]

Perhaps the example above does not use .unshift in the proper way.

The following case is more reasonable:

var rs = require('stream').Readable({ objectMode: true })
var buffer = ['a', 'b']
var unshift = false

rs._read = function () {
  if (buffer.length) {
    this.push(buffer.pop())
  }
  else {
    this.push(null)
  }
}

rs.on('data', function (chunk) {
  console.log(chunk)
  if (!unshift) {
    this.unshift('c')
    unshift = true
  }
})

output:

b
c
a

[jasnell]

So does that resolve the issue for you @zoubin ?

[zoubin]

@jasnell Almost. Thanks.

If we use stream.unshift() in stream._read, then each time we .unshift something, we have to .push(''). Right now I can remember this.

[jasnell]

Will have a pull request shortly that addresses this issue in documentation