The Annoying Closure

Today I came across some code whose behavior confused me at first. The code involved attaching onclick functions to HTML elements inside a loop. Something like this:

let divs = document.getElementsByTagName( "div" );

for ( var i = 0; i < divs.length; i++ )
{
    divs[i].onclick = function () { alert(i); }
}

There's a subtle problem with this code, which relates to closures. We'd expect to see the index of a div when we click on it; instead, no matter what div you click on, you see the total number of divs.

When the anonymous onclick function is created, it has access to the variables in parent scopes -- that's why we can access the variable i (even though it's not working the way we want). This creates a closure, which binds the variable i in the onclick function to the variable i in the outer scope (the global scope in this case, but it could just as easily be another function).

When creating a variable with var, its scope will normally be the function in which the variable was created (or else in the global scope). Modern JS also allows variables to be created with let, which behaves somewhat more complexly.

A variable created with let is bound to the nearest inner block scope -- this could be a function, an if-statement, a loop; pretty much anywhere curly braces are used. If you're familiar with C-type languages, this scoping behavior will feel very familiar.

So one solution is to simply change var to let. When the closures are created, they will bind not to a function-scoped variable, but to a block-scope variable. As best I can tell, a loop creates a new block-scope each time its body executes. In this context, the closures bind to the values we want.

let divs = document.getElementsByTagName( "div" );

for ( var i = 0; i < divs.length; i++ )
{
    divs[i].onclick = function () { alert(i); }
}

While I was researching this question, I came across the MDN article on Closures. It gets into this issue in a section called "Creating closures in loops: A common mistake."

If you're like me, you might be wondering what other solutions to the issue there are. The let keyword has only been with us for a few years. The MDN article lists a few other solutions. Two solutions introduce an intermediary function. In my simplified example, you could do the following:

let divs = document.getElementsByTagName( "div" );

for ( var i = 0; i < divs.length; i++ )
{
    (function () {
        var index = i;
        divs[i].onclick = function () { alert(index); }
    })();
}

Or, perhaps more readably, you could just give the loop-body-function a name, move it outside the loop, and call it, passing i as an argument.

The MDN article also mentions using forEach instead of a for-loop per se, which also works by creating an intermediary scope to be bound to.

I'll finish by mentioning two other approaches that the MDN article doesn't discuss.

1) You could add a property to the element the onclick function is attached to, and reference this property. For example:

let divs = document.getElementsByTagName( "div" );

for ( var i = 0; i < divs.length; i++ )
{
    divs[i].index = i;
    divs[i].onclick = function () { alert(this.index); }
}

2) You could use bind:

let divs = document.getElementsByTagName( "div" );

for ( var i = 0; i < divs.length; i++ )
{
    divs[i].onclick = (function (index) { alert(index); }).bind( divs[i], i );
}

In the above example, the element the onclick function is being attached to is passed to bind as the first parameter. This allows the function to access this if desired.

Comments

[Peter Vivo]

That problem solved the arrow functions. Because bind her closure, so in this example mouseenter function

() => console.log(`${index} :: ${div.className}`)

use index and div.className;

var index = 2387237;
const showIndexWhenHover  = (div, index) => div.addEventListener(
  'mouseenter', 
   () => console.log(`${index} :: ${div.className}`)
);
[...document.getElementsByTagName('div')].map(showIndexWhenHover);

[David Newberry]

I don't think it's the arrow function per se that's making the difference in your example, but rather the additional scope introduced by the showIndexWhenHover function. The variable 'index' in the event handler function binds to the index argument of showIndexWhenHover, rather than the global index variable.

The same code works as expected if the arrow functions are replaced with regular old function statements.

var index = 2387237;

const showIndexWhenHover  = function (div, index)  {div.addEventListener(
  'mouseenter', 
   function () { console.log(`${index} :: ${div.className}`); }
);};

([...(document.getElementsByTagName('div'))]).map(showIndexWhenHover);