pull-stream / stream-to-pull-stream

convert a node stream (classic or new) into a pull-stream
MIT License
30 stars 5 forks source link

Abort assumes close event #12

Closed skeggse closed 6 years ago

skeggse commented 6 years ago

When you abort a readable stream using stream-to-pull-stream, we assume the underlying Node-style stream will emit an close event. This is not always the case, and not required by the docs. I do think streams should emit the close event in this case, though, given the new destroy docs claim that it will emit the close event.

I'm just bringing this up in case you have a good idea on how to fix this. I'm attempting to fix this issue upstream too (by having the stream in question emit the close event).

dominictarr commented 6 years ago

I think it's a far better idea for streams to emit close consistently. Even before I went to pull-streams, I argued that node streams should consider close to be mandatory. All core streams have close, and so do streams created via through/through2 and the base classes in node. I'd consider that a defacto standard.

skeggse commented 6 years ago

Agreed, thanks for the feedback!