A huge gotcha with Javascript closures and loops
Javascript function closures probably don’t work the way you think they do.
UPDATE: I have modified my first example code to make the problem clearer.
My problem
I was writing some code today to dynamically insert a large number of HTML elements into the DOM, and give each of them a unique onclick function. My code looked vaguely like this:
[...]
var array = ...
for (int i = 0; i < length; i++) {
var object = this.createNode()
var picker = this
var x = array[i]
object.onclick = function() {
picker.select(x)
}
}
I needed to created a new function for each onclick because each one called select with a different parameter. However, I came across a problem that didn’t make any sense - when I clicked any one of the generated elements on the page, the last onclick function I had created was called. That is, if length was 31, then no matter which element I clicked, select was called with the 31st object in the array.
After hours of debugging and downloading Venkman and searching for information about Javascript closures, I finally found a discussion on the newsgroup comp.lang.javascript, in which Dom Leonard said:
Browser independant code *may not* rely on obtaining a different,
unjoined function object for *any* function declared inside another
function, whether this be because of function declaration or expression
within a loop, across calls to the containing function, or whether the
function be annonomous or named. ECMA 262 both *allows* them to be
different in all cases and *allows* them to share all external
properties (including prototype and user defined properties) in all
cases (because of join operations retaining differences in the scope
chain) and in some cases even *allows* them to be the same function
object without joining.
This was my problem - the browser appeared to be “joining” my function declarations into a single object, instead of creating a new function object for each iteration of the loop.
I’ll provide you with a solution in a minute, but first I have a fun demo for you. What do you think the following code will do?:
<html>
<head>
<script type="text/javascript">
function loadme() {
var arr = ["a", "b", "c"];
var fs = [];
for (var i in arr) {
var x = arr[i];
var f = function() { alert(x) };
f();
fs.push(f);
}
for (var j in fs) {
fs[j]();
}
}
</script>
</head>
<body onload="loadme()">
</body>
</html>
If you think it will show alerts “a” then “b” then “c” then “a” “b” “c” again, you’re wrong. In Safari and Firefox, it produces “a”, “b”, “c”, “c”, “c”, “c”. Even though we create the function() in a loop, and x is stored as part of that function’s closure, the same function is used each time, and the closure information is replaced during each iteration.
A solution
I currently use a workaround for this behavior, as described in the comp.lang.javascript discussion. The following code works correctly, producing “a” “b” “c” twice:
<html>
<head>
<script type="text/javascript">
function createFunction(x) {
return function() { alert(x); }
}
function loadme() {
var arr = ["a", "b", "c"];
var fs = [];
for (var i in arr) {
var x = arr[i];
// var f = function() {
// alert(x);
// }
var f = createFunction(x);
f();
fs.push(f);
}
for (var j in fs) {
fs[j]();
}
}
</script>
</head>
<body onload="loadme()">
</body>
</html>
However, even this code is not guaranteed to work correctly according to the ECMA spec. I don’t know what is the correct way to do this, according to the spec. If you think you’ve figured it out, please post a comment below.
August 9th, 2005 at 11:43 am
How about setting the onclick of each node to the same function, but giving the function the ability to discover at click-time, what ‘i’ should be, whether by adding an expando property at construction time, or by counting the siblings.
At any rate, the behavior you described don’t make no sense at all. fie on the browsers you tried.
Your solution seems nice. I bet what’s happening here is related to js vars being global within a block, and the js engines you tried are actually creating new closures for each iteration of the loop, but capturing the global reference. If js had the final keyword I’d suggest trying that. Reminds me of how java requires an explicit final for capturing values.
August 9th, 2005 at 12:43 pm
In your code sample “i” is variable. So when you call
picker.select(i)you are using the value of “i” when the loop has finished. You should use another closure to assign the onclick handler. Mail me for a full code sample if you need.August 9th, 2005 at 12:54 pm
Dean, unfortunately my code sample was too simple. In my actual application, it’s a new Date object which is created during each iteration. Your solution does not apply to the sample HTML file in this article, correct?
August 9th, 2005 at 1:39 pm
I would do it like this:
var f = new Function(”alert(’” + x + “‘);”);
Because, as others have said, the problem is not that this is the same function, but that all function reference the variable which changes its value.
Another option, for a more complex situations, is to store the variable x as a property of the newly created object, and access it from the function using “this”.
August 9th, 2005 at 3:14 pm
This is the logical behavior. Did you expect the closure to be a snapshot of the current state? That would have been really wierd. Given:
var x = 1;
function f () {
alert(x);
}
x = 2;
f();
I hope you did not expect this to alert “1″?
The closure contains the environment of where the function was defined. The environment consists of variables. These variables in turn point to a value.
Using new Function postpones syntax checking to runtime, instead of parse time. Using new Function is also a lot slower than using an anonymous function.
August 9th, 2005 at 3:28 pm
Erik, I’ve updated my example to more accurately reflect my problem. I believe your analysis no longer applies to the problem. The functions are now referencing a separate value each time.
August 9th, 2005 at 5:11 pm
This problem has to do with the moment variable names are dereferenced, or substituted for the values they represent. I do not think it has to do with the quote of Dom Leonard.
As you note, in your first example the variable x inside the alert()-function gets substituted only when the function is called. When the function is called in the last for-loop the value of x is set to “c”.
In your second example the variable gets effectively substituted by its value when it is passed to your new function, called createFunction(). This way the parameter of the alert() function becomes fixed and independant of global variables.
I suppose you would like to mimick this behaviour without the need of an extra function. There are two ways that I know off:
(1) var f = function (y){ return function(){ alert(y);} }(x);
In this case you use an anonymous function with one parameter called y and this function is called directly with variable x. This way the code inside the function gets executed right away. You could rename the variable y to x for obfuscation if you’d like.
(2) var f = new Function(”alert(’” + x + “‘)”);
In this case you create a function with a string. This string gets assembled directly and x is thus substituted right away. Be sure to include an extra pair of quotes so after substitution and concatenation, the string becomes for instance alert(’a') and not alert(a).
So, while this behaviour might be a gotcha, it is exactly how closures are supposed to work.
August 10th, 2005 at 2:20 am
The letters “cl” are very heavily to read e.a. in “clearer”. It looks like a “d” in Firefox 1.0.6
August 10th, 2005 at 12:51 pm
I think your createFunction solution IS the solution, and does not conflict with the ECMA spec.
Basically, your solution won’t suffer from a function declaration joining optimization. Also, it’s all about (mis?)understanding scope chains and lexical scoping. The following tweak to loadme(), for example, (when run in firefox) shows no joining is happening, but you instead get a distinct function created every time…
function loadme() {
var arr = ["a", "b", "c"];
var fs = [];
for (var i in arr) {
var x = arr[i];
var f = function() { alert(x) };
fs.push(f);
}
for (var j = 0; j
Also, playing language lawyer here on Dom Leonard's quote:
"Browser independant code *may not* rely on obtaining a different,
unjoined function object for *any* function declared inside another
function”
…It depends on the meaning of what “declared” is. For example, this is a function declaration…
function f(x) { alert(x) };
But, is this the following a function declaration? Or just syntactic sugar for new Function()?
var f = function(x) { alert(x) };
If it’s just syntactic sugar and not a real function declaration, then there won’t be any joining there. The syntactic sugar version is a lot more late bound and runtime-oriented, which a compiler can’t really predict…
var f = test ? function(x) { alert(x) } : function(x) { alert(x+1) };
That’s certainly not two declarations, just two potential runtime function creations, right?
Also, from the Leonard quote:
“join operations retain differences in the scope chain”
Even IF there was a joining optimization, the interpreter must still track the different scope chains. So, still, there’s no problem and your solution is the right one.
So, in too many bloated words, Erik’s right.
August 10th, 2005 at 12:51 pm
Previous commenters are correct.This doesn’t have anything to do with function joining. I believe that joining is something that matters only to people implementing JavaScript impelementers. To the rest of us, it should be impossible to tell whether a given implementation does it or not. (But I’m not sure about that).
Nested functions include a reference to the scope in which they are defined. But they do not make a copy of that scope. If the values of the varaibles defined in the scope change after the nested function is defined, invocations of the nested function will see the modified value. This example at my blog
http://www.davidflanagan.com/blog/2005_07.html#000065 shows two different nested functions that share a scope. One of the nested functions is a property getter and the other is a property setter. If you call the setter, it changes the value of a variable in the scope. And that change is visible through the getter function.
Any nested functions created within the same containing function invocation will share the same scope. This is not what you want in the example you present here. Your workaround is the correct one: you’ve got to have a function maker function. Each separate invocation of this function making function will create a new, independent scope, and therefore the functions it returns will be independent of each other.
August 10th, 2005 at 12:53 pm
Silly textarea — the above code should be…
function loadme() {
var arr = ["a", "b", "c"];
var fs = [];
for (var i in arr) {
var x = arr[i];
var f = function() { alert(x) };
fs.push(f);
}
for (var j = 0; j < fs.length; j++) {
if (fs[j] == fs[j - 1])
alert('hey');
}
}
September 2nd, 2005 at 6:30 pm
Comment 11 - that doesn’t work!
var arr = [”a”, “b”, “c”];
var fs = [];
for (var i in arr) {
var x = arr[i];
var f = function() { alert(x) };
fs.push(f);
}
for (var j = 0; j
October 28th, 2005 at 4:37 pm
Actually, the behavior described isn’t for either of these reasons:
“This problem has to do with the moment variable names are dereferenced, or substituted for the values they represent.” (Maarten)
“If the values of the varaibles defined in the scope change after the nested function is defined, invocations of the nested function will see the modified value.” (David Flanagan)
In a proper closure, such as the ones you find in Scheme, a closure *should* see changes made to a variable that has been closed over. But that isn’t the problem here. The problem here is that the “var” statement doesn’t create a new location. Thus,
var shared = 666;
for (var i=0; i
January 4th, 2006 at 5:07 pm
Note that this:
function createFunction(x) {
return function() { alert(x); }
}
…
var f = createFunction(x);
…is equivalent to this:
var f = (function (x) { return function () { alert (x) } })(x);
…which is the same solution that Maarten gave above. Both demonstrate the correct use of closures in this case, and are not a “workaround”. The original code is simply based on an error in understanding of what closures are supposed to do.
January 7th, 2006 at 9:42 pm
As some others have pointed out, there is nothing going on here that has to do with “joining function objects”. There are two possible reasons why this behavior might be surprising: 1) a misunderstanding of closures, or 2) the fact that `var’ declarations in JavaScript get hoisted to the top of their containing funtion.
Point (1) has been pointed out by others such as Greg and anton above. Point (2) is a subtlety of the JavaScript semantics — though a var declaration may appear to be nested within the body of the for-loop, it is actually implicitly lifted to the entire scope of the enclosing function.
The short story is that the variable `x’ in your example is allocated once in the enclosing function and then mutated at each iteration of the loop.
See: http://calculist.blogspot.com/2005/12/gotcha-gotcha.html
January 12th, 2006 at 8:57 pm
How about using eval() to turn x into a numeric literal?
January 31st, 2006 at 6:45 pm
What? Splintor has given you all the correct answer + solution, yet you continue deliberating?
Listen to what splintor says, he seems to know what he is talking about!