Skip to content

lib: place value before done in iterator results - #66837

Open
bakkot wants to merge 3 commits into
nodejs:mainfrom
bakkot:iterator-result-value-first
Open

bakkot wants to merge 3 commits into
nodejs:mainfrom
bakkot:iterator-result-value-first

Conversation

@bakkot

@bakkot bakkot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Spec requires this order, plus V8 optimizes it and not the reverse; as such this has some performance benefits (in this microbenchmark).

cc @trivikr since you added the lint rule enforcing the opposite order in #63526.

benchmark config ops/sec time per op
url/url-searchparams-iterator-consumers.js method='Map' +35.6% *** −26%
url/url-searchparams-iterator-consumers.js method='Array.from' +24.2% *** −19%
url/url-searchparams-iterator-consumers.js method='Object.fromEntries' +18.0% *** −15%
url/url-searchparams-iterator-consumers.js method='for-of' −0.9% *** +0.9%
webstreams/readable-async-iterator.js type='normal' +4.8% *** −4.6%
webstreams/readable-async-iterator.js type='bytes' +2.2% *** −2.1%
streams/readable-async-iterator.js sync='yes' +3.2% ** −3.1%
streams/readable-async-iterator.js sync='no' +1.4% * −1.4%

Assisted-by: claude:opus-5.5
Signed-off-by: Kevin Gibbons <bakkot@gmail.com>
V8 has a fast path for `{ value, done }` literals, but it must be that order.
Also, the ES spec technically requires that order.

Assisted-by: claude:opus-5.5
Signed-off-by: Kevin Gibbons <bakkot@gmail.com>
Apply the iterator-result-value-first lint fixer.

Assisted-by: claude:opus-5.5
Signed-off-by: Kevin Gibbons <bakkot@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/streams
  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants