stream: cleanup use of _readableState.ended #29645
Conversation
nodejs-github-bot
added
http2
Does that mean that e.g. the |
|
I don't this so. As per the readable stream current implementation, any subsequent |
addaleax
left a comment
addaleax
left a comment
There was a problem hiding this comment.
LGTM but I’d feel more comfortable if there was a test somewhere (whether it already exists or not) that makes sure that repeated .push(null) calls do not lead to errors :)
|
Yes, makes sense. I will look for one if exists or add it as part of this PR if missing. Thanks. |
|
I couldn't find an existing test verifying multiple |
trivikr
added
the
author ready
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
|
The subsystem prefix on the second commit should have been |
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #29645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
9 participants
Replaces references to Readable stream's internal state
_readableState.endedwithreadableEnded.One thing to highlight that readable stream internally (L#216)
sets the
readableEndedproperty using_readableState.endEmittedandnot
_readableState.ended. The files changed in this PR used_readableState.endedstate.Although all existing tests pass and it seems correct to rely on the
_readableState.endEmittedto know when the stream ended, pleasesuggest if this could be a potential issue.
cc: @addaleax @mcollina
Refs: #445
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes