Why is for loop setting all jquery onclick to last iteration

Question

I have the following code that works

var btns = $('.gotobtn');
$('#'+btns.get(0).id).click(function() {
    document.querySelector('#navigator').pushPage('directions.html', myInfo[0]); });

$('#'+btns.get(1).id).click(function() {
    document.querySelector('#navigator').pushPage('directions.html', myInfo[1]); });

$('#'+btns.get(2).id).click(function() {
    document.querySelector('#navigator').pushPage('directions.html', myInfo[2]); });

// this works. I click on button 0 and get myInfo[0], 
// on 1 and get myInfo[1], on 2 and get myInfo[2]

But replacing it with a loop does not work correctly. Instead, I always get the last element: myIfno[2] for any button I press.

var btns = $('.gotobtn');
var i = 0;
for (i = 0; i<3; i++){
    var btnid = "#" + btns.get(i).id;
    $(btnid).click(function() {                        
        document.querySelector('#navigator').pushPage('directions.html', myInfo[i]); });
    }
    // this does set the buttons on-click but when I click on them, 
    // all get the latest iteration, in this example myInfo[2]

Why is this? And how do I fix that, without defining each button manually? I want to see how to do it in jquery.

Thanks Ibrahim, but if I understand that answer, in my case it only explains why, but does not help me solve it. The reason its happening is because I'm binding the onclick to i, which at the end of the iterations is 3. So I am binding to the same i each time. If I add a function btnclick(i) would that not still bind to my variable? Wait, I'm testing, and if so, will tell you its duplicate. — pashute, Sep 04 '17 at 14:44
No, because javascript creates a closure for each iteration by calling the function `btnclick(i)`. In your code there is only one closure which is shared by all iterations. A closure is created within each function call. In your code all iteration share a reference to the same `i` variable and since it get incremented, they will all get its last value. In the `btnclick(i)` version, each iteration will have a reference to its own version of the `i` variable (not the `i` used in the loop but the `i` that get copied as parameter when you call `btnclick`). — ibrahim mahrir, Sep 04 '17 at 14:48
The problem is that javascript doesn't respect **block scope** as most languages do. `if(true) { var i = 5; } console.log(i);` is valid and prints `5` in javascript, but throws an error in other languages that respect the block scope **`{ }`**. — ibrahim mahrir, Sep 04 '17 at 14:51
My favorite answer from the duplicate link are [**this one**](https://stackoverflow.com/a/750495/6647153) and the one that uses `forEach` instead of `for` loop. — ibrahim mahrir, Sep 04 '17 at 14:56
Also, you can easily make this work with `.bind()` - rather than declare your anonymous function to have 0 parameters, declare it to take `i` as a parameter, and use `bind` to supply that parameter ahead of time. The one caveat with `bind` is that it will also change the value of `this`, but if you're not using `this`, there's no specific concern here. — PMV, Sep 04 '17 at 14:59

score 0 · Answer 1 · answered Sep 04 '17 at 14:42

0

Because: JavaScript does not have block scope. Variables introduced with a block are scoped to the containing function or script

Replace:

$(btnid).click(function() {                        
        document.querySelector('#navigator').pushPage('directions.html', myInfo[i]); 
});

With:

$(btnid).click(customFunction(i));

And declare this function outside the loop:

function customFunction(i) {                        
     document.querySelector('#navigator').pushPage('directions.html', myInfo[i]); 
 }

answered Sep 04 '17 at 14:42

Sletheren

2,435
11
25

This doesn't work, because this will execute `customFunction(i)` immediately when it attempts to register the handler, and the actual handler would become undefined because that is what `customFunction()` returns. You could use a similar approach, but `customFunction(i)` would need to actually return a function - you'd be applying parameters; you'd create a function with 1 parameter that returns a function with 0 parameters suitable to be used as a callback. – PMV Sep 04 '17 at 14:55

score 0 · Answer 2 · answered Sep 04 '17 at 14:48

0

your

$(btnid).click(function() {                        
        document.querySelector('#navigator').pushPage('directions.html', myInfo[i]); });

Must be outside your for loop.

answered Sep 04 '17 at 14:48

Sebastian S

807
6
15

Thank you. This is correct, but was not clear enough. You didn't explain WHY it's happening, and not HOW exactly the $(btnid).click() would be used "outside the loop". For that reason I wrote the full answer. Sorry. – pashute Sep 11 '17 at 10:39

pashute · Accepted Answer · 2017-09-11T10:48:14.010

@ibrahim mahrir is correct. It's caused by the same phenomena as described in 46039325 although this question is specific for JQuery binding and will probably be useful to some. (I saw the unanswered question in several places on the web)

It happens because I'm binding to i, and in the meantime i has changed to the last iteration (which is 2 in this example). I need to bind to the value of i while iterating.

This will happen (due to quirks in javascript) if I define the binding to a parameter of a function. The parameter is "dynamically created" each time and the value of that param (during that iteration) will be bound.

So when I finally do click on the second button (id:1, the first is id:0), it will invoke the method with the value of 1, correctly.

Here's an example of how the fix looks in jQuery:

$(function(){ // document ready
    function btnaction(i){
        var btns = $('.gotobtn');
        $('#'+btns.get(i).id).click(function() {
            document.querySelector('#navigator').pushPage('directions.html', gotoInfo[i]);
        });
    }

and I call it in the loop

for (i = 0; i<6; i++)
    btnaction(i);

Alls well that ends well...

Why is for loop setting all jquery onclick to last iteration

3 Answers3