"ESLint no-loop-func rule" What to do when loop variables are required in a callback

Question

The company where I work for requires us to follow the no-loop-func ES-lint rule. I am in a situation where a loop variable is required in a callback.

An example can be found bellow:

var itemsProcessed = 0;
for (var index = 0; index < uniqueIdentifiers.length; index++) {
  let uniqueIdentifier = uniqueIdentifiers[index];

  // ESLint: Don't make functions within a loop (no-loop-func)
  var removeFileReferenceCallback = function (removeReferenceError) {
    if (removeReferenceError !== null) {
      NotificationSystem.logReferenceNotRemoved(uniqueIdentifier, undefined);
    }

    // When all items are removed, use the callback
    if (++itemsProcessed === uniqueIdentifiers.length) {
      callback(null);
    }
  };

  // Remove the reference
  this.database.removeFileReference(uniqueIdentifier, removeFileReferenceCallback);
}

How can the code be refactored so that the rule can be met?

Create a function that accepts those as parameters and returns another function. — zerkms, Apr 29 '16 at 07:55
@zerkms That would only work if the variable passed in to the function is passed by value. This works: `for (var i = 0; i < 10; i++) {funcs[i] = ((i) => () => i)(i)}`. This doesn't because we are passing by reference: `const obj = {}; for (var i = 0; i < 10; i++) {obj.i = i; funcs[i] = ((obj) => () => obj.i)(obj)}`. — 0xcaff, Jul 06 '17 at 04:01

georg · Accepted Answer · 2016-04-29T08:28:41.463

12

Just don't use loops. Iterator methods are so much better.

uniqueIdentifiers.forEach(function(uid) {
    this.database.removeFileReference(uid, function (err) {
        if (err) {
            NotificationSystem.logReferenceNotRemoved(uid, undefined);
        }
    });
}, this);

callback(null);

To call a callback after everything has been completed, you're going to need something like this:

var db = self.database;

var promises = uniqueIdentifiers.map(function(uid) {
    return new Promise(function (res) {
        db.removeFileReference(uid, function (err) {
            if (err) {
                NotificationSystem.logReferenceNotRemoved(uid, undefined);
            }
            res();
        });
    });
});

Promise.all(promises).then(callback);

edited Apr 29 '16 at 08:28

answered Apr 29 '16 at 08:00

georg

211,518
52
313
390

1

`callback` in their code is potentially (most likely) invoked asynchronously. – zerkms Apr 29 '16 at 08:01
@georg Very nice alternative way, but the callback has to be called when all the references are removed. – Laurence Apr 29 '16 at 08:04
2

@Laurence `var itemsLeft = uniqueIdentifiers.length;` then `if (--itemsLeft === 0) { callback(null); }` in a callback function. – zerkms Apr 29 '16 at 08:05
I can't do what I want to do without a loop because I use something like a cursor to use data from the previous iteration, so what is actually the loop-y way to do this? – Umagon Nov 25 '22 at 02:30

score 0 · Answer 2 · edited May 23 '17 at 11:52

0

I use ESList for all my JavaScript and I adhere to the no-loop-func rule, it will bite you badly if you don't. The reason is well explained here: JavaScript closure inside loops – simple practical example.

I suggest the other answers in terms of refactors.

edited May 23 '17 at 11:52

Community

1
1

answered Apr 29 '16 at 08:07

psiphi75

1,985
1
24
36

This would throw an error about undeclared `removeReferenceError` variable. – zerkms Apr 29 '16 at 08:08

score 0 · Answer 3 · answered Apr 29 '16 at 08:16

Could using bind solve your problem?

var itemsProcessed = 0;
for (var index = 0; index < uniqueIdentifiers.length; index++) {
    let uniqueIdentifier = uniqueIdentifiers[index];

    // Remove the reference
    this.database.removeFileReference(uniqueIdentifier, removeFileReferenceCallback.bind(uniqueIdentifier));
}

function removeFileReferenceCallback(removeReferenceError) {
    if (removeReferenceError !== null) {
        NotificationSystem.logReferenceNotRemoved(this, undefined);
    }

    // When all items are removed, use the callback
    if (++itemsProcessed === uniqueIdentifiers.length) {
        callback(null);
    }
};

`bind` also creates a new function. In a loop. http://www.ecma-international.org/ecma-262/6.0/#sec-function.prototype.bind — zerkms, Apr 29 '16 at 08:17

score 0 · Answer 4 · answered Mar 31 '20 at 04:10

0

I also converted my object into array and then used forEach.

answered Mar 31 '20 at 04:10

QauseenMZ

1,081
8
6

"ESLint no-loop-func rule" What to do when loop variables are required in a callback

4 Answers4