fix: eliminate for ... of syntax by himself65 · Pull Request #59279 · nodejs/node

himself65 · 2025-07-29T23:26:54Z

  | Type                  | Node v24.4.1      | Release Build     | Change |
  |-----------------------|-------------------|-------------------|--------|
  | Raw (5 listeners)     | 166,550,996 ops/s | 168,262,835 ops/s | +1.0%  |
  | Wrapped (5 listeners) | 83,660,424 ops/s  | 88,460,347 ops/s  | +5.7%  |
  | Raw (50 listeners)    | 45,603,881 ops/s  | 47,712,200 ops/s  | +4.6%  |

Inspired from #59278

himself65

not going to make this ready, looks like some parts of change is unnecessary

aduh95 · 2025-07-30T06:57:26Z

lib/internal/abort_controller.js

+        const sourceSignalWeakRefs = ArrayFrom(signal[kSourceSignals]);
+        for (let i = 0; i < sourceSignalWeakRefs.length; i++) {
+          const sourceSignalWeakRef = sourceSignalWeakRefs[i];


That's almost certainly less efficient, ArrayFrom iterate over the object, so we essentially double the number of iteration

the loop should be converted to a callback, the second argument of ArrayFrom

I would just leave the for..of here tbh, but in any case, it should be a separate PR

aduh95 · 2025-07-30T06:59:23Z

lib/internal/crypto/keys.js

+const encodings = [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'],
+                   [kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']];
+for (let i = 0; i < encodings.length; i++) {
+  const m = encodings[i];
  encodingNames[m[0]] = m[1];
+}


I would try to remove that loop entirely at this point

Suggested change

const encodings = [[kKeyEncodingPKCS1, 'pkcs1'], [kKeyEncodingPKCS8, 'pkcs8'],

[kKeyEncodingSPKI, 'spki'], [kKeyEncodingSEC1, 'sec1']];

for (let i = 0; i < encodings.length; i++) {

const m = encodings[i];

encodingNames[m[0]] = m[1];

}

encodingNames[kKeyEncodingPKCS1] = 'pkcs1';

encodingNames[kKeyEncodingPKCS8] = 'pkcs8';

encodingNames[kKeyEncodingSPKI] = 'spki';

encodingNames[kKeyEncodingSEC1] = 'sec1';

yeah, make sense. we could move to a separate PR

aduh95 · 2025-07-30T07:00:17Z

lib/internal/fs/cp/cp.js

+  let entry;
+  while ((entry = await dir.read()) !== null) {
+    const { name } = entry;


This would need to be evaluated separately

aduh95

If you make this PR focused on array iteration (i.e. if you remove all the ArrayFrom calls and leave other non-array iteration for a separate PR), it'd be much easier to review and approve.
I'm +1 on the idea, it also aligns with our recommendation in

node/doc/contributing/primordials.md

Lines 152 to 215 in 9239373

    
           <summary>Avoid for-of loops on arrays</summary> 
        
           ```js 
        
           for (const item of array) { 
        
             console.log(item); 
        
           } 
        
           ``` 
        
           This code is internally expanded into something that looks like: 
        
           ```js 
        
           { 
        
             // 1. Lookup %Symbol.iterator% property on `array` (user-mutable if 
        
             //    user-provided). 
        
             // 2. Lookup %Symbol.iterator% property on %Array.prototype% (user-mutable). 
        
             // 3. Call that function. 
        
             const iterator = array[Symbol.iterator](); 
        
             // 1. Lookup `next` property on `iterator` (doesn't exist). 
        
             // 2. Lookup `next` property on %ArrayIteratorPrototype% (user-mutable). 
        
             // 3. Call that function. 
        
             let { done, value: item } = iterator.next(); 
        
             while (!done) { 
        
               console.log(item); 
        
               // Repeat. 
        
               ({ done, value: item } = iterator.next()); 
        
             } 
        
           } 
        
           ``` 
        
           Instead of utilizing iterators, you can use the more traditional but still very 
        
           performant `for` loop: 
        
           ```js 
        
           for (let i = 0; i < array.length; i++) { 
        
             console.log(array[i]); 
        
           } 
        
           ``` 
        
           The following code snippet illustrates how user-land code could impact the 
        
           behavior of internal modules: 
        
           ```js 
        
           // User-land 
        
           Array.prototype[Symbol.iterator] = () => ({ 
        
             next: () => ({ done: true }), 
        
           }); 
        
           // Core 
        
           let forOfLoopBlockExecuted = false; 
        
           let forLoopBlockExecuted = false; 
        
           const array = [1, 2, 3]; 
        
           for (const item of array) { 
        
             forOfLoopBlockExecuted = true; 
        
           } 
        
           for (let i = 0; i < array.length; i++) { 
        
             forLoopBlockExecuted = true; 
        
           } 
        
           console.log(forOfLoopBlockExecuted); // false 
        
           console.log(forLoopBlockExecuted); // true 
        
           ``` 
        
           This only applies if you are working with a genuine array (or array-like 
        
           object). If you are instead expecting an iterator, a for-of loop may be a better 
        
           choice.

meta: add no-unsafe-array-iteration.js rule

947a949

nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jul 29, 2025

fix: eliminate for ... of syntax

0ef363b

himself65 changed the title ~~meta: add no-unsafe-array-iteration.js rule~~ fix: eliminate for ... of syntax Jul 30, 2025

himself65 commented Jul 30, 2025

View reviewed changes

aduh95 reviewed Jul 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: eliminate for ... of syntax#59279

fix: eliminate for ... of syntax#59279
himself65 wants to merge 2 commits intonodejs:mainfrom
himself65:himself65/2025/07/29/eslint-rule

himself65 commented Jul 29, 2025 •

edited

Loading

Uh oh!

himself65 left a comment

Uh oh!

aduh95 Jul 30, 2025

Uh oh!

ljharb Jul 30, 2025

Uh oh!

aduh95 Jul 30, 2025

Uh oh!

aduh95 Jul 30, 2025 •

edited

Loading

Uh oh!

himself65 Jul 30, 2025

Uh oh!

aduh95 Jul 30, 2025

Uh oh!

aduh95 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	<summary>Avoid for-of loops on arrays</summary>

	```js
	for (const item of array) {
	console.log(item);
	}
	```

	This code is internally expanded into something that looks like:

	```js
	{
	// 1. Lookup %Symbol.iterator% property on `array` (user-mutable if
	// user-provided).
	// 2. Lookup %Symbol.iterator% property on %Array.prototype% (user-mutable).
	// 3. Call that function.
	const iterator = array[Symbol.iterator]();
	// 1. Lookup `next` property on `iterator` (doesn't exist).
	// 2. Lookup `next` property on %ArrayIteratorPrototype% (user-mutable).
	// 3. Call that function.
	let { done, value: item } = iterator.next();
	while (!done) {
	console.log(item);
	// Repeat.
	({ done, value: item } = iterator.next());
	}
	}
	```

	Instead of utilizing iterators, you can use the more traditional but still very
	performant `for` loop:

	```js
	for (let i = 0; i < array.length; i++) {
	console.log(array[i]);
	}
	```

	The following code snippet illustrates how user-land code could impact the
	behavior of internal modules:

	```js
	// User-land
	Array.prototype[Symbol.iterator] = () => ({
	next: () => ({ done: true }),
	});

	// Core
	let forOfLoopBlockExecuted = false;
	let forLoopBlockExecuted = false;
	const array = [1, 2, 3];
	for (const item of array) {
	forOfLoopBlockExecuted = true;
	}
	for (let i = 0; i < array.length; i++) {
	forLoopBlockExecuted = true;
	}
	console.log(forOfLoopBlockExecuted); // false
	console.log(forLoopBlockExecuted); // true
	```

	This only applies if you are working with a genuine array (or array-like
	object). If you are instead expecting an iterator, a for-of loop may be a better
	choice.

Uh oh!

Conversation

himself65 commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

himself65 left a comment

Choose a reason for hiding this comment

Uh oh!

aduh95 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

ljharb Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

aduh95 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

aduh95 Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himself65 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

aduh95 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

himself65 commented Jul 29, 2025 •

edited

Loading

aduh95 Jul 30, 2025 •

edited

Loading