-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: add readable and writable property to Readable and Writable #23933
doc: add readable and writable property to Readable and Writable #23933
Conversation
Co-Authored-By: dexterleng <[email protected]>
ping @jasnell |
@nodejs/streams @nodejs/documentation This could use some reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value is slightly different, and the meaning as well. writable
is true
when it's safe to call write()
, and readable
is true
when it's safe to call read()
.
The problem is that some of the core and ecosystem streams sets readable
or writable
to false
to take into account a delayed open scenario.
@mcollina Could you elaborate? In the constructor of Readable and Writable the properties are both true. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR-URL: nodejs#23933 Reviewed-By: Matteo Collina <[email protected]>
Landed in 16a2b5c. Thanks for the contribution! 🎉 |
PR-URL: #23933 Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #23933 Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #23933 Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#23933 Reviewed-By: Matteo Collina <[email protected]>
FWIW this landed with |
PR-URL: #23933 Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #23933 Reviewed-By: Matteo Collina <[email protected]>
Issue #21431
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes