Skip to content

tools: enforce iterator result property order#63526

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:eslint-iterator-result-object-done-value
Open

tools: enforce iterator result property order#63526
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:eslint-iterator-result-object-done-value

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 24, 2026

Adds a custom ESLint rule to enforce done before value in iterator result objects.

This keeps iterator result object literals consistent across lib/, and includes a fixer
that swaps the properties when they are in the wrong order.

The existing iterator result object literals in lib/ have been updated to match the new rule.

Refs: #63521 (comment)


Assisted-by: openai:gpt-5.5

Add a custom ESLint rule requiring iterator result objects to place
`done` before `value`, and update existing lib iterator result objects
to follow the rule.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams
  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 24, 2026
@trivikr trivikr requested a review from jasnell May 24, 2026 00:42
@codecov
Copy link
Copy Markdown

codecov Bot commented May 24, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (837910d) to head (d25477b).

Files with missing lines Patch % Lines
lib/internal/streams/iter/push.js 75.00% 2 Missing ⚠️
lib/internal/vfs/watcher.js 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #63526   +/-   ##
=======================================
  Coverage   90.33%   90.33%           
=======================================
  Files         730      730           
  Lines      234205   234205           
  Branches    43927    43931    +4     
=======================================
+ Hits       211559   211560    +1     
+ Misses      14370    14366    -4     
- Partials     8276     8279    +3     
Files with missing lines Coverage Δ
lib/events.js 99.59% <100.00%> (ø)
lib/internal/fs/promises.js 92.87% <100.00%> (ø)
lib/internal/url.js 93.17% <100.00%> (ø)
lib/internal/webstreams/readablestream.js 98.54% <100.00%> (ø)
lib/internal/vfs/watcher.js 95.20% <83.33%> (ø)
lib/internal/streams/iter/push.js 92.20% <75.00%> (ø)

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 24, 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 related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants