-
Notifications
You must be signed in to change notification settings - Fork 30k
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
eos and stream type detection #29395
Comments
You are correct. You cannot desume if something is going to be readable/writable in the future by looking at those properties, only if it could be readable/writable now. I think we should improve eos in this regard. |
I think we can assume that is something a readable and is not destroyed it will become readable at some point. I think what we primarily need to improve is how we detect whether something is |
The value of stream.readable and stream.writable should not be used to detect whether a stream is Writable or Readable. Refs: nodejs#29395
The value of stream.readable and stream.writable should not be used to detect whether a stream is Writable or Readable. Refs: #29395 PR-URL: #29409 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
This has been fixed. |
The value of stream.readable and stream.writable should not be used to detect whether a stream is Writable or Readable. Refs: #29395 PR-URL: #29409 Backport-PR-URL: #31345 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
The value of stream.readable and stream.writable should not be used to detect whether a stream is Writable or Readable. Refs: #29395 PR-URL: #29409 Backport-PR-URL: #31345 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Looking at https://github.com/nodejs/node/blob/master/lib/internal/streams/end-of-stream.js#L31
It looks like "eos" uses the
writable
andreadable
properties to detect whether a stream isReadable
like orWritable
like. However, according my current incomplete understanding of these properties (#29377), they cannot/should not be used for this purpose, i.e. aReadable
stream could in theory have areadable = false
which later becomes areadable = true
.Should we maybe change these lines to something like e.g:
The text was updated successfully, but these errors were encountered: