addEventListener on loop not working with IIFE

Question

In the code below, the value of j is always set to the last of the iteration. I'm already using an IIFE, but it still gives the non-expected behavior. Is there something that I'm doing wrong?

function renderUsers(users) {
    const frag = document.createDocumentFragment();
    const userList = document.getElementById('user-list');

    // Create and append all the users on the user list
    for (var j = 0; j < users.length; j++) {
        var item = document.createElement('li');
        var division = document.createElement('div');
        var userName = document.createElement('span');
        var deleteButtonAnchor = document.createElement('a');
        var deleteButton = document.createElement('i');
        deleteButton.classList.add('material-icons');
        deleteButton.textContent = 'delete_forever';
        (function() {
            deleteButton.addEventListener('click',function() {
                console.log(j);
            });
        })();
        deleteButtonAnchor.appendChild(deleteButton);
        division.appendChild(userName);
        division.appendChild(deleteButtonAnchor);
        item.appendChild(division);


        userName.appendChild(document.createTextNode(users[j].name.first+' '+users[j].name.last));
        frag.appendChild(item);
    }
    userList.appendChild(frag);
}

You're not passing `j` to the IIFE. Creating functions in a loop is also considered as bad practice. Use external function instead. See http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Teemu, Jan 16 '17 at 18:34

Adam Wolski · Accepted Answer · 2017-01-16T19:51:55.567

2

What you want to do with closure, is to pass the iterating variable as a parameter, as such:

 (function(j) {
     deleteButton.addEventListener('click',function() {
          console.log(j); // j is the parameter
     });
 })(j);

Or as @torazaburo fairly noticed in the comments, you can use let keyword for a iterating variable to eliminate the need to create closures. More on the let keyword you can find here. Note however, that it is ES6 feature, which is not implemented in older browsers, so you may need to transpile it first (for example using babel).

edited Jan 16 '17 at 19:51

answered Jan 16 '17 at 18:35

Adam Wolski

5,448
2
27
46

Because there's no need for a closure here at all, since the entire problem can be solved with `for (let...`. – Jan 16 '17 at 19:51
Sure, but the question was " Is there something that I'm doing wrong?". This answer says what was wrong in the provided code. – Adam Wolski Jan 16 '17 at 19:52
1

Since the OP is already using `const`, it is safe to assume that he either is targeting ES6 browsers, or transpiling. Anyway, "what he is doing wrong" is using a closure when he doesn't need to at all. – Jan 16 '17 at 19:55
That is one way to look at this question, another is "why this closure does not work". Using ES6 is best practice for sure, however knowing what is behind the new features, and how things work (such as principal concept of JS - closure) is a good thing. Don't you agree? – Adam Wolski Jan 16 '17 at 19:59
Sure, but this technique of using IIFE's in functions using the index value within loops is, in an ES6 world, largely of academic interest, and this should be pointed out (as you rightly did in your edit). Let's consider the hypothetical case of a user who did not know the `+` operator, only `++` and `--`. He is trying to add two numbers, and has written a function to add *n* to a number using recursion, as in `function add(x, n) { return n ? add(x, n--) : x; }`. Certainly rather than helping him debug that by changing it to `--n`, a "correct" solution would be to suggest he use `x + n`. – Jan 16 '17 at 21:05
Well, you are assuming that the question was not asked for learning purposes. Maybe he or she tries to learn closures (OP did not specified). I understand your analogy but I find it misplaced, you can’t compare advanced concepts to primitive operators. It is common to implement such concepts in the process of learning, just for the sake of understanding. Knowing how fn scope works in JS is crucial to write code well, and scope’s consequences extend beyond ‘use let instead of closures in loops’. – Adam Wolski Jan 16 '17 at 22:08
Therefore, a "correct" solution imo would be to correct provided code, help to understand the context and comment about the current best practises in particular context (loop), which is why I edited the answer accordingly. – Adam Wolski Jan 16 '17 at 22:08

addEventListener on loop not working with IIFE

1 Answers1