13

If I call jQuery.ajax() inside a loop, would it cause the call in current iteration overwrite the last call or a new XHR object is assigned for the new request?

I have a loop that do this, while from console log I can see requests done 200 ok but just the result data of the last request in the loop is stored by the request success callback as supposed .

the code:

var Ajax = {
    pages: {},

    current_request: null,

    prefetch: function () {
        currentPath = location.pathname.substr(1);

        if(this.pages[currentPath])
        {
            var current = this.pages[currentPath];
            delete this.pages[currentPath];

            current['name']=currentPath;
            current['title']=$("title").text().replace(' - '.SITE_NAME, '');
            current['meta_description']=$("meta[name=description]").attr('content');
            current['meta_keywords']=$("meta[name=keywords]").attr('content');          
        }

        var _Ajax = this;
        //the loop in question *****
        for(var key in this.pages)
        {
            $.ajax({
                method: 'get',
                url:'http://'+location.hostname+'/'+key,
                success: function(data) {
                    _Ajax.pages[key] = data;    
                }
            }); 

                    console.debug(this.pages);
        }

        if(current)
        {
            this.pages[currentPath] = current;
        }       

    } 
};//Ajax Obj
for(var i in pages)
{
    Ajax.pages[pages[i]]={};
}

$(function() {
    Ajax.prefetch();
});//doc ready
MTVS
  • 2,046
  • 5
  • 26
  • 37
  • 2
    Multiple ajax calls will be handled appropriately, but if you are assigning values to a variable defined outside of the scope of the callback, that may get overwritten. Care to post some code? – Jason P Jul 31 '13 at 21:53
  • Ajax in a regular loop is always a dangerous due to ajax being asynchronous. Can we see some code? – musicnothing Jul 31 '13 at 21:54

5 Answers5

29

You'll need a closure for key:

for(var k in this.pages){
    (function(key){
            $.ajax({
                method: 'get',
                url:'http://'+location.hostname+'/'+key,
                success: function(data) {
                    _Ajax.pages[key] = data;    
                }
            }); 

            console.debug(this.pages);
    })(k);
}

that way you make sure that key is always the correct on in each ajax success callback. but other than that it should work

i made a small closure demonstration using timeout instead of ajax but the principle is the same:

http://jsfiddle.net/KS6q5/

Andy
  • 29,707
  • 9
  • 41
  • 58
  • Yes, thank you this works very good but I think its more clearer to add the key to the `$.ajax options argument` and accessing it using `this` keyword inside the `success` callback – MTVS Jul 31 '13 at 22:42
12

You need to use async:false in you ajax request. It will send the ajax request synchronously waiting for the previous request to finish and then sending the next request.

$.ajax({
    type: 'POST',
    url: 'http://stackoverflow.com',
    data: data,
    async: false,
    success: function(data) {
        //do something
    }, 
    error: function(jqXHR) {
        //do something
    }
});
Shubham Khatri
  • 270,417
  • 55
  • 406
  • 400
Yura Zabolotny
  • 121
  • 1
  • 2
6

I believe what's happening here has to do with closure. In this loop:

    for(var key in this.pages)
    {
        $.ajax({
            method: 'get',
            url:'http://'+location.hostname+'/'+key,
            success: function(data) {
                _Ajax.pages[key] = data;    
            }
        }); 

                console.debug(this.pages);
    }

The variable key is actually defined outside the for loop. So by the time you get to the callbacks, the value has probably changed. Try something like this instead:

http://jsfiddle.net/VHWvs/

var pages = ["a", "b", "c"];

for (var key in pages) {
    console.log('before: ' + key);
    (function (thisKey) {
        setTimeout(function () {
            console.log('after: ' + thisKey);
        }, 1000);
    })(key);
}
Jason P
  • 26,984
  • 3
  • 31
  • 45
  • Your answer should be correct, but in practice it produced the same result, does thisKey also get overwritten? – MTVS Jul 31 '13 at 22:11
  • i don't think this will work as expected since it's not a closure. so `thiskey` will probably always be the last one for each ajax callback. – Andy Jul 31 '13 at 22:17
  • Looks like I got the problem, but not the solution. Here's a solution that works, though I don't know if it's the best: http://jsfiddle.net/VHWvs/ – Jason P Jul 31 '13 at 22:18
  • I added the thisKey to the $.ajax options argument and it worked, thanks, you may edit your answer so I can select as accepted answer. – MTVS Jul 31 '13 at 22:20
3

This is how I always do a ajax loop..

I use a recursive function that gets called after the xhr.readyState == 4

i = 0
process()
function process() {
    if (i < 10) {
        url = "http://some.." + i
        var xhr = new XMLHttpRequest();
        xhr.open("GET", url, true);
        xhr.onreadystatechange = function () {
            if (xhr.readyState == 4) {
                alert(xhr.responseText)
                i++
                process()
            }
        }
        xhr.send();
    } else {
        alert("done")
    }
}
Varun Sridharan
  • 1,983
  • 2
  • 20
  • 54
gezzuzz
  • 188
  • 2
  • 16
  • Your function is running the requests one at a time which means it will likely run much slower. His original function was running them in parallel. – idbehold Jul 31 '13 at 22:22
  • 2
    true.. this is purposely set to run 1 at a time.. avoids a bottle neck opening 100's of request at once.. really depends whats he trying to do.. if he opening only a few request then doing them all at once is fine.. also if you dont care what order the request come back.. because the the 3rd request could finish before the first.. – gezzuzz Aug 01 '13 at 00:16
3

I was facing the same situation, I solved using the ajax call inside a new function then invoke the function into the loop.

It would looks like:

function a(){
    for(var key in this.pages)
    {
      var paramsOut [] = ...
      myAjaxCall(key,paramsOut);
      .......
    }
}
function myAjaxCall(paramsIn,paramsOut)
{
       $.ajax({
        method: 'get',
        url:'http://'+location.hostname+'/'+paramsIn[0],
        success: function(data) {
            paramsOut[key] = data;    
        }
      }); 
}
JPRLCol
  • 749
  • 11
  • 28