Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readable event,outputs are different between v9.11 and v10.14.2 #25158

Closed
curry-wxj opened this issue Dec 21, 2018 · 10 comments
Closed

readable event,outputs are different between v9.11 and v10.14.2 #25158

curry-wxj opened this issue Dec 21, 2018 · 10 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@curry-wxj
Copy link

curry-wxj commented Dec 21, 2018

1.txt 内容为123456789,
代码:
const rs = fs.createReadStream(path.join(__dirname, './1.txt'), {
highWaterMark: 3,
encoding: 'utf8'
})
rs.on('readable', () => {
let ch = rs.read(1)
console.log('ch', ch);
})
9.11版本node 输出 ch 1(readable触发了1次)
10.14.2版本 输出 ch 1 ch 2 (readable触发了2次)
不太懂 readable 什么情况下会再次触发,

@ZYSzys
Copy link
Member

ZYSzys commented Dec 21, 2018

Maybe /cc @nodejs/streams , if I'm not wrong, it seems that this is relevant to #17979.#21690.

const fs = require('fs');
const path = require('path');
// the content of `1.txt` is just `123456789`
const rs = fs.createReadStream(path.join(__dirname, './1.txt'), {
  highWaterMark: 3,
  encoding: 'utf8'
})
rs.on('readable', () => {
  let ch = rs.read(1)
  console.log('ch', ch);
})

The outputs are different between v9.11 and v10.14.2:

Node.js v9.11:

ch 1

Nodej.js v10.14.2:

ch 1
ch 2

@shangchihh
Copy link

@curry-wxj In English please. Let other guys can understand you. 用英语描述吧,让其他人能够看懂你的描述。

@curry-wxj curry-wxj changed the title 绑定 readable事件,node版本不同,执行结果不同 readable event,outputs are different between v9.11 and v10.14.2 Dec 24, 2018
@lpinca lpinca added the stream Issues and PRs related to the stream subsystem. label Dec 28, 2018
@ryzokuken
Copy link
Contributor

I'm sorry, but could someone please translate? I don't think this will get a lot of traction as-is.

@ZYSzys
Copy link
Member

ZYSzys commented Dec 28, 2018

@ryzokuken Hey, please look at my comment above, the example code above has different outputs between Node.js v9 and v10.

It seems that the readable event is emitted twice in v10.

@ryzokuken
Copy link
Contributor

@ZYSzys @curry-wxj the same behavior can be observed in master. I'm not quite up to speed with the streams subsystem, but seems like someone made a fix, it was backported to v10 but not v9 (since v9 has hit EOL).

Nobody should use v9 at this point anyway. If you feel like the current behavior (as observed on v10 and master) is a bug, feel free to make an issue for that.

@ryzokuken
Copy link
Contributor

@ZYSzys I guess this could be closed since the only reason the commit wasn't backported to v9 is because it is EOL?

I'd leave that up to you. Either way, the OP could reopen if they feel dissatisfied.

@ZYSzys
Copy link
Member

ZYSzys commented Dec 28, 2018

@ryzokuken The behavior on v8 LTS is the same as v9 which only emitted once(Oh, the commit isn’t backported to v8), but the v10(and master) is weird, IMHO.

I indeed thought it’s a bug on v10 and master, if I’m not wrong 🤔.

@ryzokuken
Copy link
Contributor

Maybe @nodejs/streams could tell.

@ZYSzys
Copy link
Member

ZYSzys commented Jan 10, 2019

ping @nodejs/streams again.
Did the documentation change in #25375 fix this issue enough?

@mcollina
Copy link
Member

Yes, I think so.

@targos targos closed this as completed Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants