Skip to content

Conversation

@BenClaveau
Copy link

No description provided.

return destroyer(stream, reading, writing, function (err) {
if (!error) error = err
if (err) destroys.forEach(call)
if (err) destroys.forEach(fn => fn(err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No arrow functions

@mafintosh
Copy link
Owner

The reason we don't do this is because destroy behavior with an argument is pretty undefined in node land (until very recent node changes). Lots of streams out in the wild will do different things, some expect the argument to be an optional cb fx.

In node master there has recently been some good improvements on standardizing the behavior so we could start doing this in the future :)

@BenClaveau
Copy link
Author

@mafintosh ok I understand but in nodejs source https://github.com/nodejs/node/blob/master/lib/internal/streams/destroy.js error is the first argument not a cb.

@mafintosh
Copy link
Owner

@BenoitClaveau yea, that's the improvements in node core I was talking about. we still support older versions, plus other stream types so at the current moment we should still call it without and error to be sure. If you wanna feature detect somehow that it's a newer node core stream (and it's simple enough) I'd be ok adding it.

@BenClaveau
Copy link
Author

Great. If I understand I need to know if the stream is a node stream3. Is that right ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants