Repository navigation
Behavior of readablestreams changed in v10 #20520
Description
Activity
The provided example is not a good way of using streams, because it will add a new
'data'handler for each emitted chunk (even on Node < 10). In thecp-fileit will callresolveevery time there is a new chunk.The solution (also to the cp-file case) is to use
once()instead ofon(). See sindresorhus/copy-file#26.I think this can be closed.
Reacted by Luigi Pincait will call
resolveevery time there is a new chunk.That is not a problem, because of the semantics of Promises. You may call
resolveas often as you want, only the first one matters:new Promise((resolve, reject) => { resolve('resolve: ' + 1); resolve('resolve: ' + 2); resolve('resolve: ' + 3); reject('reject: ' + 1); reject('reject: ' + 2); reject('reject: ' + 3); }) .then(console.log) .catch(console.error); // outputs on node v9 and v10 resolve: 1
Please also note, that your suggestion (using
oncefordataevents) does not solve the problem in the example of @tmcw:const fs = require('fs'); const read = fs.createReadStream(__filename); read.on('readable', () => { read.once('data', chunk => { console.log('data'); }); }); // nodejs v9 //=> data // nodejs v10 //=> (no output)
@mcollina Interestingly, your suggestion to use
.onceinstead of.onforreadableevents seems to solve the issues forcp-fileand the example above:read.once('readable', () => { read.on('data', chunk => { console.log('data'); }); }); // nodejs v9 and v10 //=> data
Thus, it seems
.on('readable')and.once('readable')behave differently.That is not a problem, because of the semantics of Promises. You may call resolve as often as you want, only the first one matters
Yes but it's useless overhead.
Please also note, that your suggestion (using once for data events) does not solve the problem in the example of @tmcw:
The suggestion was to use
oncefor'readable', not for'data'.The behavior change introduced in #18994 is semver-major but it seems to work as intended.
Reacted by Michael MayerThe behavior change introduces in #1899 is semver-major but it seems to work as intended.
That PR is 3 years old and rather long. Moreover the discussion does not contain the words
read,readableorstream– Where do I have to look?I'm not sure what your point is. It is unexpected, if
.on('readable')and.once('readable')behave differently, isn't it? (except for the differences mentioned at Handling events only once of course 😉 )It was a typo when referencing the issue. The "4" was missing at the end. I fixed that by editing the reference.
@BridgeAR Thank you, for clarifying. But, please do not edit my comments in a way, so the discussion does not make sense for readers anymore.
@schnittstabil my point is that cf5f986 seems to work as expected, specifically:
This make
.resume()a no-op if there is a listener for the
'readable'event, making the stream non-flowing if there is a
'data'listener.Ok, but both
.on('readable', …)and.once('readable', …)create listeners for the
'readable'event, don't they? – I believe, I still don't understand 😕The difference is that if there is at least one listener for the
'readable'event,on('data', fn)will not resume the stream.once()in the above example makes sure that there are no listeners whenon('data', fn)is called.
Hi Maintainers!
This may be the same underlying issue as #20503 - I'm not confident enough in the node core space enough to tell.
This problem was first noticed in cp-file, a dependency of cpy, which is a dependency of one of my projects. A very distilled test case is:
This is a program that reads itself, by creating a read stream, waiting for it to be readable, and then assigning a data event listener. It prints
dataon all node versions < 10, but prints nothing on the Node v10.0.0 release.In situ, cp-file tries to ensure that a stream is readable before creating the destination directory and then calling
.pipeto write the file to disk. As far as I can tell, Node v10 changes the behavior of readable streams such that data events are not emitted if you bind the data event listener after readable is emitted.