Jquery: .load() with for loop, always takes the element

Question

Im have a jquery code that running in loop over my images and load them one by one, All works fine beside the fact that it inserting to the 'a' tag the last image in results! Here is my code (Please take a look in "var a"... it should insert the variable 'thissrc' differently every loop:

$.get(url, function (data) {
    var countImages = data.length;
    console.log("count: " + countImages);
    var $ul = $('#thumbs').empty();
    var counter = 0;
    for (var i = 0; i < countImages; ++i) {
        try {
            var description = data[i].desc[0];
        } catch (e) {
            description = '';
        }
        if (description == undefined) description = '';
        var thissrc = data[i].src;
        console.log("ME: " + thissrc);
        $('<img width="' + screen.width / 2.4 + '" alt="' + data[i].alt + '" title="' + description + '"/>').load(function () {
            ++counter;
            var $this = $(this);
            var $li = $('<span/>', {
                className: 'pic'
            });
            var $a = $('<a/>', {
                href: '#',
                    'id': 'rel',
                    'data-inline': true,
                    'data-transition': 'slide',
                className: 'show-page-loading-msg',
                    'data-referrer': 'photo_container',
                    'data-imgsrc': thissrc
            });
            $ul.append($li.append($a.append($this)));
            if (counter == countImages) {
                $thumbscontainer.append($ul.show());
            }
        }).attr('src', data[i].src);

    }
}, 'json');

Thanks in Advance!

Eran.

score 0 · Accepted Answer · answered Jun 08 '13 at 19:49

You should be able to set data-imgsrc by reading back the image's src, ie :

'data-imgsrc': this.src

This will be reliable assuming no other code has had the opportunity to change the src in the short while after the img was created and before its initial src successfully loads.

If this is not a safe assumption then the solution is somewhat more elaborate but not at all uncommon.

score 0 · Answer 2 · answered Jun 08 '13 at 19:52

A common mistake. The problem boils down to:

for (var i = 0; i < countImages; ++i) {
    var thissrc = data[i].src;
    setTimeout(function() {
        // Will only ever alert the last value of thissrc
        alert(thissrc);
    }, 100);
}

In JavaScript, a for construct (or any regular block statement) does not create a new lexical scope. That is, each iteration uses the same variable thissrc. The above snippet is equivalent to:

var thissrc;
for (var i = 0; i < countImages; ++i) {
    thissrc = data[i].src;
    setTimeout(function() {
        alert(thissrc);
    }, 100);
}

In fact, every variable declaration (using var) inside a function is scoped to that function.

MDN explains the problem really well and provides a solution using an extra function. In this case, it could look like:

$.get(url, function (data) {
    // ...
    for (var i = 0; i < countImages; ++i) {
        // ...
        var thissrc = data[i].src;
        // ...
        // Make a new callback for $.load using the *current* value of thissrc
        $.load(url, makeCallback(thissrc));
    }
    // ...

    function makeCallback(thissrc) {
        // Yes, this function returns another function
        return function() {
             // Do your callback stuff here
        };
    }
}

Note that because makeCallback is defined inside $.get's callback function, it has access to all those local variables such as counter and description. Those variables are in the scope of makeCallback.

JavaScript 1.7 makes this a bit easier with the let keyword:

let allows you to declare variables, limiting its scope to the block, statement, or expression on which it is used. This is unlike the var keyword, which defines a variable globally, or locally to an entire function regardless of block scope.

That means that you could fix your problem with just one change:

let thissrc = data[i].src;

Unfortunately, as this is a fairly recent addition, it's not supported in old browsers.

Thanks a ton for this! However, I noticed something - I am creating divs in a loop, loading each div with HTML content using .load and then populating the fields inside the newly created div. I noticed that these divs get populated in the reverse order of creation. i.e. the last created div is populated first and the div created first is populated last. Is there some recursion happening here? Will it be usable without issues of memory leaks in case the for loop is really long? — Anurag Joshi, Sep 20 '14 at 13:03
@AnuragJoshi No, there's no recursion in this pattern, it's probably something else in your code. I'd suggest making a new question for your problem with full details and code, you're probably not going to get much help in the comments of this old question (and it's kind of off-topic). — Mattias Buelens, Sep 20 '14 at 21:28

Jquery: .load() with for loop, always takes the element

2 Answers2