Conversation
|
// @Richienb |
index.js
Outdated
| next.reject(signal.reason); | ||
| // Resolve pending promises when iterator is closed | ||
| // Only reject when aborted via signal | ||
| for (const {resolve, reject} of resolvers.splice(0)) { |
There was a problem hiding this comment.
.splice(0) ?
It would probably be nicer to reject all inside abort callback, and resolve all inside return(), and then both call cleanup().
There was a problem hiding this comment.
Do you mean like this? bbc2356 I think the previous version was slightly nicer.
There was a problem hiding this comment.
.splice(0) ?
Intentional. Atomically grabs and clears the queue, avoids O(n) shift() loops and re-entry races.
There was a problem hiding this comment.
Do you mean like this?
Yes!
I think this version is better because we don't need an if statement anymore, so simpler. Specifically, when I go to run this code in my head, there's less to reason about.
The previous version centralised everything under a single cleanup function. But simplicity over centralisation.
No description provided.