Having trouble with translation from jQuery to Vanilla JS, doesn't work when there is multiple elements

Question

I'm having trouble with translating jQuery to Vanilla JS, here is my code in jQuery:

$("[target]").change(function () {
    var attr = $(this).attr("target");
    this.obj[attr] = this.value
});

And I decided to translate it to Vanilla JS, to get bettor performances (I also improved it a bit), and I came with that:

var addEventListener = (function() {
    if(document.addEventListener) {
        return function(element, event, handler) {
             element.addEventListener(event, handler, false);
         };
    }
    else {
        return function(element, event, handler) {
             element.attachEvent('on' + event, handler);
        };
    }
}());
var elements = document.querySelectorAll("[target]");
var obj = this.obj;
for (var i = 0; i < elements.length; i++) {
    var attr = elements[i].getAttribute("target");
    addEventListener(elements[i], "change", function () {
        obj[attr] = this.value;
    });
    addEventListener(elements[i], "keydown", function () {
        obj[attr] = this.value;
    });
    addEventListener(elements[i], "input", function () {
        obj[attr] = this.value;
    });
    addEventListener(elements[i], "paste", function () {
        obj[attr] = this.value;
    });
}

The problem is that it works, but for only 1 element, if there is 2 or more element with the attribute "target", then, it just doesn't work properly. Any idea to fix this?

Just to say, I'm using a class to integrate this code, so the `this.obj` refers to a variable that I declared in the constructor — dev.arguiot, May 01 '17 at 03:12
The problem is the `attr` variable, and the fact that all four event listeners on all of your elements use the *same* `attr` variable. — nnnnnn, May 01 '17 at 03:20
"And I decided to translate it to Vanilla JS, to get bettor performances" .... why? What reason do you have to believe that *you actually need to do this*. It seems to me that you are sacrificing a ton of readability for an extremely minor performance gain that you really dont need. — Wesley Smith, May 01 '17 at 03:24

score 0 · Answer 1 · edited May 23 '17 at 11:33

The following bit of code might be "unclean" or "improper" in some other way, but this is how I normally deal with node list traversal:

[].forEach.call(document.querySelectorAll('[target]'), function(x, i, a) {
    addEventListener(a[i], 'eventType', function() {
        //whatever
    });
});

I would've personally given the addEventListener function a different name to differentiate it from the inbuilt one.

P.S. This might help you further: JavaScript closure inside loops – simple practical example

P.P.S. I don't think using Vanilla JS over jQuery offers a significant advantage, but that's an opinion as well.

Having trouble with translation from jQuery to Vanilla JS, doesn't work when there is multiple elements

1 Answers1