javascript - Debugging: ESLint Warning "Function declared in a loop contains unsafe references to variable(s)...no-loop-func"

Question

Building a Sort-Visualizer in React using the Create-React-App [https://roy-05.github.io/sort-visualizer/ ]

I'm animating each iteration of the loop using setTimeouts. On dev console I get the following warning:

Line 156:32: Function declared in a loop contains unsafe references to variable(s) 'minimum', 'minimum', 'minimum', 'minimum' no-loop-func

Here's the code-snippet:

for(let i=0; i<arr.length-1; i++){
            let minimum = i; //Declare minimum here
            setTimeout(()=>{
                for(let j = i+1; j<arr.length; j++){
                    setTimeout(()=>{
                        //Getting a warning for these references:
                        array_bar[j].style.backgroundColor = 'red';
                        array_bar[minimum].style.backgroundColor = 'blue';
                        setTimeout(()=>{
                            if(arr[j] < arr[minimum]){
                            array_bar[minimum].style.backgroundColor = 'lightblue';
                            minimum = j; 
                            }  
                            else{
                                array_bar[j].style.backgroundColor = 'lightblue';
                            }  
                        }, 4);
                    }, (j-1)*4);    
                }

Going through ESLint Docs, I believe the issue might be that i'm modifying the value inside the setTimeout but the variable is declared outside its scope.

I'm not sure how to fix that warning, any help will be appreciated!

Note: Here's the entire function if you need it -

selectionSort(){
        const arr = this.state.array,
            array_bar = document.getElementsByClassName("array-elem");

        this.setState({startedSelectionSort: true});

        for(let i=0; i<arr.length-1; i++){
            let minimum = i; //Declare minimum here
            setTimeout(()=>{
                for(let j = i+1; j<arr.length; j++){
                    setTimeout(()=>{
                        //Getting a warning for these references:
                        array_bar[j].style.backgroundColor = 'red';
                        array_bar[minimum].style.backgroundColor = 'blue';
                        setTimeout(()=>{
                            if(arr[j] < arr[minimum]){
                            array_bar[minimum].style.backgroundColor = 'lightblue';
                            minimum = j; 
                            }  
                            else{
                                array_bar[j].style.backgroundColor = 'lightblue';
                            }  
                        }, 4);
                    }, (j-1)*4);    
                }
                setTimeout(()=>{
                    let temp = arr[i],
                    arr1_height = arr[minimum],
                    arr2_height = arr[i];

                    arr[i] = arr[minimum];
                    arr[minimum] = temp;

                    array_bar[i].style.height = `${arr1_height}px`;
                    array_bar[minimum].style.height = `${arr2_height}px`;

                    array_bar[i].style.backgroundColor = "green";
                    if(i !== minimum){
                        array_bar[minimum].style.backgroundColor = 'lightblue';
                    }
                }, 400);


                if(i === arr.length-2){
                    setTimeout(()=>{
                        array_bar[i+1].style.backgroundColor = "green";
                    },800);
                }

            }, i*400);
        }

        setTimeout(()=>{
            this.setState({sorted: true})
        }, arr.length*400+1750);

    }

Answer 1

I also encountered same warning. In my case, I declared variable outside the iteration, but modified variable inside forEach method.

Something like:

// some code above
let validInputs = true;

someInputs.forEach( input => {
  validInputs = input.value && validInputs;
})

After I did some reserch, I found in this post, JSHint error : Functions declared within loops referencing an outer scoped variable may lead to confusing semantics, mentioned that JSHint doesn't like how the anonymous function in there is being re-created over and over.

I changed forEach arrow function to for (let index i = 0; index < someInputs.length; index++), and the warning is gone.

Perhaps in your case, change setTimeout to traditional non-arrow function can remove the warning.

updated on Apr 7th 2021

As I'm reading the Professional JavaScript for Web Developers, 4th edition, I might have found why this warning is implemented in the ESLint.

From section 4.3 Garbage Collection sections, the book mentioned that closure might also lead to memory leak.

The purpose for forEach and arrow function is to limit the scope of the variable, as describes below from MDN:

Arrow functions establish "this" based on the scope the Arrow function is defined within. from Arrow function expressions

In section Creating closures in loops: A common mistake, MDN mentioned:

Another alternative could be to use forEach() to iterate over the helpText array and attach a listener to each , as shown:

function showHelp(help) {
  document.getElementById('help').textContent = help;
}

function setupHelp() {
  var helpText = [
      {'id': 'email', 'help': 'Your e-mail address'},
      {'id': 'name', 'help': 'Your full name'},
      {'id': 'age', 'help': 'Your age (you must be over 16)'}
    ];

  helpText.forEach(function(text) {
    document.getElementById(text.id).onfocus = function() {
      showHelp(text.help);
    }
  });
}

setupHelp();

In our implementation, calling arrow functions inside forEach is creating closure of closure, which obviously can create some confusing semantics for garbage collection.