4

I have a bunch of files to iterate over. For each file, depending on some of the field values, I may or may not need to make asynchronous AJAX requests to get additional data. After all these requests have resolved I want to append the relevant data for that file to the web page before continuing to the next file. My actual code contains irrelevant info, but the logic resembles the following pseudocode:

var chain = $.when();
for(x = 0; x < files.length; x++) {
    console.log('FOO');
    chain = chain.then(function() {
        var fieldNamePromises = [];
        var fieldName2Promises = [];

        if ('fieldName' in files[x]) {
            //getFieldNameData returns an AJAX request
            fieldNamePromises.push(getFieldNameData())
        }

        if ('fieldName2' in files[x]) {
            //getField2NameData returns an AJAX request
            fieldName2Promises.push(getFieldName2Data())
        }

        Promise.all(fieldNamePromises.concat(fieldName2Promises)).then(function() {
            console.log('BAR');
        })
    });
}

For me, this would log:

'FOO'
'FOO'
'FOO'
'BAR'
'BAR'
'BAR'

Whereas I want:

'FOO'
'BAR'
'FOO'
'BAR'
'FOO'
'BAR'

How can I force it to wait for all my asynchronous requests to finish before continuing in my loop?

The following also produces three FOOs followed by three BARs instead of FOO BAR alternating.

getFileHistory(fileNumber, function (files) {
    var chain = $.when();
    for (let x = 0; x < files.length; x++) {
        chain = chain.then(function () {
            console.log('FOO');
            var fieldNamePromises = [];

            fieldNamePromises.push(getData('Data', function () { }))


            fieldNamePromises.push(getData('Data', function () { }))


            return Promise.all(fieldNamePromises).then(function () {
                console.log('BAR');
            })
        });
    }
});

function getFileHistory(fileNumber, callback) {
    var url = window.baseUrl + "api/get-file-history/filter?fileNumber=" + fileNumber;
    return $.ajax({
        type: 'GET',
        url: url,
        contentType: 'application/json; charset=utf-8',
        dataType: "json",
        success: function (result) {
            callback(result);
        }
    });
}

function getData(id, callback) {
    var url = "api/" + id;
    return $.ajax({
        type: 'GET',
        url: url,
        contentType: 'application/json; charset=utf-8',
        dataType: "json",
        success: function (result) {
            callback(result);
        },
        error: function (error) {
            console.log(error);
        }
    });
}

Update: when I remove the loop from getFileHistory (another AJAX call), it returns the FOO/BARs as expected. Not entirely sure why this makes a difference.

Evan Hessler
  • 297
  • 3
  • 19
  • 1
    While an Array Order is guaranteed, each AJAX call might take different time to return. i would suggest chaining them and not executing them all at once. chain as in `callOne.then(callTwo.then(callThree))` – Rafael Herscovici Mar 14 '19 at 17:05
  • Since each AJAX call may or may not need be called, building a chain like this will be quite ugly, if at all possible (actual code is far more intricate). Wondering if there is a cleaner alternative. – Evan Hessler Mar 14 '19 at 17:09
  • @Dementic The following code still returns 'FOO FOO BAR BAR' instead of 'FOO BAR FOO BAR' - any ideas why? for (x = 0; x < files.length; x++) { console.log('FOO'); getData('Data').then(getData2('Data2')).then(function () { console.log('BAR'); }); } – Evan Hessler Mar 14 '19 at 18:11
  • Agreed it would be ugly, but i dont think you have another option. – Rafael Herscovici Mar 14 '19 at 19:43
  • Your FOOs are logged synchronously (in your for loop) and your BARs asynchronously (in later event threads), guaranteeing `FOO, FOO, FOO, BAR, BAR, BAR`. For `FOO, BAR, FOO, BAR, FOO, BAR`, move the line `console.log(foo)` inside the outer `then()` and, equally importantly, `return Promise.all(...).then(...)`. Without `return`, the chain is not informed of the settlement of `Promise.all(...).then(...)`. – Roamer-1888 Mar 14 '19 at 23:32
  • @Roamer-1888, I believe I have tried this (see my edit) but I'm still getting the same result. Mind taking a look? – Evan Hessler Mar 15 '19 at 15:25
  • Your second snippet should in fact work. Did you try out exactly this? – Bergi Mar 15 '19 at 15:44
  • Your first snippet has the [common closure in a loop problem](https://stackoverflow.com/q/750486/1048572), using `x` asynchronously in the `then` callback. Is this an issue in your actual code as well? Fix it by using `for (let x = …`. – Bergi Mar 15 '19 at 15:45
  • @Bergi, looking into the common closure problem. That may be part of the issue. As for the second snippet, I've edited to add more context, but yes that is in fact what I am running. – Evan Hessler Mar 15 '19 at 15:58
  • @EvanHessler What is `getFileHistory`? How often does it call your callback, and how long is the `files` array? – Bergi Mar 15 '19 at 16:16
  • @EvanHessler Also, most important: [what version of jQuery are you using?](https://stackoverflow.com/a/32480918/1048572) – Bergi Mar 15 '19 at 16:17
  • @Bergi, I've added getFileHistory. It definitely seems to be the source of the problem as the loop behavior is expected without it. Right now only 5 files are returned in getFileHistory. Also, I am using Jquery 3.3.1 – Evan Hessler Mar 15 '19 at 16:30
  • Hm, I can't see how `getFileHistory` would contribute to the problem. To follow best practices though, you should drop the `callback` parameter and simply use the returned promise with `then`. – Bergi Mar 15 '19 at 16:42
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/190104/discussion-between-evan-hessler-and-bergi). – Evan Hessler Mar 15 '19 at 16:50

1 Answers1

0

try creating a different function for it an async like below

    async function(){
         var chain = $.when();
        for(x = 0; x < files.length; x++) {
            console.log('FOO');
            chain = await chain.then(async() => {
                var fieldNamePromises = [];
                var fieldName2Promises = [];

                if ('fieldName' in files[x]) {
                    //getFieldNameData returns an AJAX request
                    fieldNamePromises.push(getFieldNameData())
                }

                if ('fieldName2' in files[x]) {
                    //getField2NameData returns an AJAX request
                    fieldName2Promises.push(getFieldName2Data())
                }

               await Promise.all(fieldNamePromises.concat(fieldName2Promises)).then(function() {
                    console.log('BAR');
                })
            });
        }
  }
Yushin
  • 1,684
  • 3
  • 20
  • 36
  • 1
    Thanks. I should have mentioned that I did this and it worked. ... but I need this to work for IE and async/await isn't supported. – Evan Hessler Mar 15 '19 at 15:40
  • 1
    No, don't do this. Avoid [mixing `await` and `.then(…)` syntax](https://stackoverflow.com/a/54387912/1048572)! If you want to use `async`/`await`, do it thoroughly and drop the `chain` altogether. – Bergi Mar 15 '19 at 15:47