Skip to content

Comments

Minor simplifications#2

Merged
sindresorhus merged 2 commits intomainfrom
simplify
Sep 7, 2025
Merged

Minor simplifications#2
sindresorhus merged 2 commits intomainfrom
simplify

Conversation

@sindresorhus
Copy link
Owner

No description provided.

@sindresorhus
Copy link
Owner Author

// @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)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

.splice(0) ?

It would probably be nicer to reject all inside abort callback, and resolve all inside return(), and then both call cleanup().

Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you mean like this? bbc2356 I think the previous version was slightly nicer.

Copy link
Owner Author

Choose a reason for hiding this comment

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

.splice(0) ?

Intentional. Atomically grabs and clears the queue, avoids O(n) shift() loops and re-entry races.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

@sindresorhus sindresorhus merged commit 0a139e8 into main Sep 7, 2025
4 checks passed
@sindresorhus sindresorhus deleted the simplify branch September 7, 2025 18:29
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.

2 participants