Issue in rewriting scripts to be vanilla js (not jQuery)

Question

I've been rewriting my Chrome Extension in vanilla code rather than jQuery, and for the most part it's been easy. I have however come across a part that doesn't work as intended.

Original jQuery-based code:

$(".content .flair").click(function(){
    //Irelevant code using "this"
});

New Vanilla-only code:

var flairs = document.querySelectorAll(".content .flair");
for (var i = 0; i < flairs.length; i++){
    flairs[i].addEventListener("click", function(event){
        //Irrelevant code using flairs[i]
    });
}

The issue with the vanilla code is that every click on a .content .flair element always uses information from the last element in flairs (i.e. flairs[flairs.length - 1] rather than using flairs[i].

I just can't squash this bug!

see http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Arun P Johny, Apr 22 '14 at 05:39
and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Creating_closures_in_loops.3A_A_common_mistake — Arun P Johny, Apr 22 '14 at 05:39
@ArunPJohny I'm amazed why is this happening? Umm, what about declaring var outside the for loop ? — Bhojendra Rauniyar, Apr 22 '14 at 05:45
possible duplicate of [Why is the loop assigning a reference of the last index element to?](http://stackoverflow.com/questions/21487187/why-is-the-loop-assigning-a-reference-of-the-last-index-element-to) — jfriend00, Apr 22 '14 at 06:25

score 0 · Accepted Answer · edited May 23 '17 at 12:11

0

Issue was with closures.

See SO - Closures and MDN - Closures with thanks to @Arun-P-Johny

edited May 23 '17 at 12:11

Community

1
1

answered Apr 22 '14 at 05:46

GusGold

470
5
18

@ArunPJohny Cheers. It's for a chrome extension, so no need for IE support :) – GusGold Apr 22 '14 at 06:29
then go with the second version – Arun P Johny Apr 22 '14 at 06:31
@ArunPJohny Since `flairs` is a `nodeList` and not an `array`, `forEach` isn't an available method, so I'm just using the first version [see MDN NodeList](https://developer.mozilla.org/en/docs/Web/API/NodeList#Why_can't_I_use_forEach_or_map_on_a_NodeList.3F) – GusGold Apr 22 '14 at 07:54

score 0 · Answer 2 · answered Apr 22 '14 at 18:51

A great option for dealing with this is to turn your DOM element list into a proper Array using slice:

var buttonElementList = document.querySelectorAll('button');
// not a real array, so .forEach will not work

var buttonElementArray = Array.prototype.slice.call(buttonElementList);
// returns an array, can call .forEach or .map (or use .length)

Issue in rewriting scripts to be vanilla js (not jQuery)

2 Answers2