Commit 3930128a104 for nodejs
commit 3930128a104fc69cde41c03be72a8ba383154f71
Author: Kevin Gibbons <bakkot@gmail.com>
Date: Sun Oct 18 13:46:06 2026 -0700
tools: require `value` before `done` in iterator results
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>
PR-URL: https://github.com/nodejs/node/pull/66837
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs
index 9240b00ac70..fcd4dd4aa78 100644
--- a/lib/eslint.config_partial.mjs
+++ b/lib/eslint.config_partial.mjs
@@ -435,7 +435,7 @@ export default [
'node-core/alphabetize-errors': 'error',
'node-core/alphabetize-primordials': 'error',
'node-core/avoid-prototype-pollution': 'error',
- 'node-core/iterator-result-done-first': 'error',
+ 'node-core/iterator-result-value-first': 'error',
'node-core/lowercase-name-for-primitive': 'error',
'node-core/non-ascii-character': 'error',
'node-core/no-array-destructuring': 'error',
diff --git a/test/parallel/test-eslint-iterator-result-done-first.js b/test/parallel/test-eslint-iterator-result-done-first.js
deleted file mode 100644
index f65f6bd72eb..00000000000
--- a/test/parallel/test-eslint-iterator-result-done-first.js
+++ /dev/null
@@ -1,60 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if ((!common.hasCrypto) || (!common.hasIntl)) {
- common.skip('ESLint tests require crypto and Intl');
-}
-
-common.skipIfEslintMissing();
-
-const { RuleTester } = require('../../tools/eslint/node_modules/eslint');
-const rule = require('../../tools/eslint-rules/iterator-result-done-first');
-
-const message = 'Iterator result objects should place `done` before `value`.';
-
-new RuleTester().run('iterator-result-done-first', rule, {
- valid: [
- 'function next() { return { done: true, value: undefined }; }',
- 'function next() { return { __proto__: null, done: false, value: chunk }; }',
- 'function next() { return { done, value }; }',
- 'function next() { return { value }; }',
- 'function next() { return { done }; }',
- 'function next() { return { value: 1, other: 2 }; }',
- 'function next() { return { [value]: 1, done: true }; }',
- 'function next() { return { value: 1, [done]: true }; }',
- 'function next() { return { "done": true, "value": undefined }; }',
- 'function next() { return { ["done"]: true, ["value"]: undefined }; }',
- ],
- invalid: [
- {
- code: 'function next() { return { value: undefined, done: true }; }',
- errors: [{ message }],
- output: 'function next() { return { done: true, value: undefined }; }',
- },
- {
- code: 'function next() { return { __proto__: null, value: chunk, done: false }; }',
- errors: [{ message }],
- output: 'function next() { return { __proto__: null, done: false, value: chunk }; }',
- },
- {
- code: 'function next() { return { value, done }; }',
- errors: [{ message }],
- output: 'function next() { return { done, value }; }',
- },
- {
- code: 'function next() { return { "value": undefined, "done": true }; }',
- errors: [{ message }],
- output: 'function next() { return { "done": true, "value": undefined }; }',
- },
- {
- code: 'function next() { return { ["value"]: undefined, ["done"]: true }; }',
- errors: [{ message }],
- output: 'function next() { return { ["done"]: true, ["value"]: undefined }; }',
- },
- {
- code: 'function next() { return { value: result, extra: true, done: false }; }',
- errors: [{ message }],
- output: 'function next() { return { done: false, extra: true, value: result }; }',
- },
- ],
-});
diff --git a/test/parallel/test-eslint-iterator-result-value-first.js b/test/parallel/test-eslint-iterator-result-value-first.js
new file mode 100644
index 00000000000..24dc56be143
--- /dev/null
+++ b/test/parallel/test-eslint-iterator-result-value-first.js
@@ -0,0 +1,60 @@
+'use strict';
+
+const common = require('../common');
+if ((!common.hasCrypto) || (!common.hasIntl)) {
+ common.skip('ESLint tests require crypto and Intl');
+}
+
+common.skipIfEslintMissing();
+
+const { RuleTester } = require('../../tools/eslint/node_modules/eslint');
+const rule = require('../../tools/eslint-rules/iterator-result-value-first');
+
+const message = 'Iterator result objects should place `value` before `done`.';
+
+new RuleTester().run('iterator-result-value-first', rule, {
+ valid: [
+ 'function next() { return { value: undefined, done: true }; }',
+ 'function next() { return { __proto__: null, value: chunk, done: false }; }',
+ 'function next() { return { value, done }; }',
+ 'function next() { return { value }; }',
+ 'function next() { return { done }; }',
+ 'function next() { return { done: 1, other: 2 }; }',
+ 'function next() { return { [done]: 1, value: true }; }',
+ 'function next() { return { done: 1, [value]: true }; }',
+ 'function next() { return { "value": undefined, "done": true }; }',
+ 'function next() { return { ["value"]: undefined, ["done"]: true }; }',
+ ],
+ invalid: [
+ {
+ code: 'function next() { return { done: true, value: undefined }; }',
+ errors: [{ message }],
+ output: 'function next() { return { value: undefined, done: true }; }',
+ },
+ {
+ code: 'function next() { return { __proto__: null, done: false, value: chunk }; }',
+ errors: [{ message }],
+ output: 'function next() { return { __proto__: null, value: chunk, done: false }; }',
+ },
+ {
+ code: 'function next() { return { done, value }; }',
+ errors: [{ message }],
+ output: 'function next() { return { value, done }; }',
+ },
+ {
+ code: 'function next() { return { "done": true, "value": undefined }; }',
+ errors: [{ message }],
+ output: 'function next() { return { "value": undefined, "done": true }; }',
+ },
+ {
+ code: 'function next() { return { ["done"]: true, ["value"]: undefined }; }',
+ errors: [{ message }],
+ output: 'function next() { return { ["value"]: undefined, ["done"]: true }; }',
+ },
+ {
+ code: 'function next() { return { done: false, extra: true, value: result }; }',
+ errors: [{ message }],
+ output: 'function next() { return { value: result, extra: true, done: false }; }',
+ },
+ ],
+});
diff --git a/tools/eslint-rules/iterator-result-done-first.js b/tools/eslint-rules/iterator-result-value-first.js
similarity index 68%
rename from tools/eslint-rules/iterator-result-done-first.js
rename to tools/eslint-rules/iterator-result-value-first.js
index 1c3ee9035e9..09d6fbe3027 100644
--- a/tools/eslint-rules/iterator-result-done-first.js
+++ b/tools/eslint-rules/iterator-result-value-first.js
@@ -1,6 +1,10 @@
'use strict';
-const MESSAGE = 'Iterator result objects should place `done` before `value`.';
+// V8 has a fast path for `{ value, done }` literals, but it must be that order.
+// https://source.chromium.org/chromium/chromium/src/+/main:v8/src/init/bootstrapper.cc;l=746-754;drc=e8ac4914ee6b24442aed773d3a041451692b6a9c
+// Also, the ES spec technically requires that order.
+// https://tc39.es/ecma262/multipage/abstract-operations.html#sec-createiterresultobject
+const MESSAGE = 'Iterator result objects should place `value` before `done`.';
function getStaticPropertyName(property) {
const { key } = property;
@@ -48,9 +52,9 @@ module.exports = {
}
}
- if (doneProperty && valueProperty && valueProperty.range[0] < doneProperty.range[0]) {
+ if (doneProperty && valueProperty && doneProperty.range[0] < valueProperty.range[0]) {
context.report({
- node: valueProperty,
+ node: doneProperty,
message: MESSAGE,
fix(fixer) {
return [