Promises not resolved in a .each() loop

Question

I've got to run a recursive process and the promises are not working as I want. This is the code:

var openAllLeves = function () {
    openAll = 1;
    $.when(openLevels()).then(openAll = 0);
}

var openLevels = function () {
    var promises = [];     
    $('.myClass:not([data-type="T"])').each(function () {
        var defer = $.Deferred();
        $.when(loadLine($(this)).then(promises.push(defer)));
    });
    return $.when.apply(undefined, promises).promise();
}

var loadLine = function (thisObj) {
    var defer = $.Deferred();
    switch(nivel) {
        case 1:
            $.when(getPT($(thisObj).attr('data-a'))).then(defer.resolve());
            break;
        case 2:
            // ...
    }
    return defer.promise();
}

var getPT = function (psn) {
    var defer = $.Deferred();
    var payload = { /* parameters... */ };
    $.ajax({
        url: webmethod,
        data: payload,
        type: "POST",
        contentType: "application/json; charset=utf-8",
        dataType: "json",
        timeout: 10000,
        success: function (data) {
            $.when(paintPT(data)).then(function () { 
                if (openAll) 
                    openLevels(), defer.resolve(); 
            });
        }
    });
    return defer.promise();
}

My problem is that openAll's value changes to 0 before being evaluated in the ajax function success code so only one iteration is performed and the recursivity is not done. It looks like .then is performed before resolving the array of promises.

The code is a little bit confusing so any help is appreciated. Thanks in advance.

Just trying to perform an ajax call foreach row in a table. This calls return another rows and they generate new calls until the returned row has the attribute 'data-type="T"'. Thanks for your interest. — MKP, Aug 04 '16 at 10:38
can't you generate the table in the serverside language ?, if you can why aren't you doing it? — madalinivascu, Aug 04 '16 at 10:43
The recursivity is made in the success of the ajax call calling openLevels() if openAll is true. So sorry but my requirements don't allow me to generate it in the serverside. Big thanks for your interest. — MKP, Aug 04 '16 at 10:55
what requirements don't allow you to generate the table in the back-end?, i think you have a xy problem here — madalinivascu, Aug 04 '16 at 11:11
Project requirements. The new rows can be loaded as this example or one by one fired by an event. — MKP, Aug 04 '16 at 11:20

Tiago Engel · Answer 1 · 2016-08-04T19:47:25.563

1

One big problem in your code is that you're calling the functions on the then callback and not passing them to it. For instance:

.then(defer.resolve());

This way you are passing the value of defer.resolve() to the then callback and not the function that should be called when the async action finished. You should be doing something like this:

.then(defer.resolve.bind(defer));

The same applies for the rest of the code. You should take a look at the promise spec

Particularly

If onFulfilled is not a function, it must be ignored.

EDIT

As pointed out by Bergi you should avoid the deferred antipattern.

edited Aug 04 '16 at 19:47

answered Aug 04 '16 at 12:03

Tiago Engel

3,533
1
17
22

That's not the real problem. – Bergi Aug 04 '16 at 13:00
How so? "My problem is that openAll's value changes to 0 before being evaluated in the ajax function " I think that's exactly the problem.. I didn't say the only problem was in this specific line, It was only an example. – Tiago Engel Aug 04 '16 at 16:57
"*The problem is …*" sounds like it is the problem that when fixed makes the code work correctly. However, the deferred antipattern is a much more prevalent problem, and not fixing it is wrong. – Bergi Aug 04 '16 at 19:07
yes, I agree the deferred antipattern is bad and your answer is better. But still, even using a bad pattern if he fixes the problem I pointed out the problem will be fixed (setting openAll to 0 before the ajax request). I don't see the point in downvoting my answer. – Tiago Engel Aug 04 '16 at 19:37

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Avoid the deferred antipattern!

Also, when you pass something to .then(), it must be callback function, calling promises.push(defer), defer.resolve() and openAll = 0 or so does not work, it would execute that expression right away instead of waiting for the promise.

The $.when() and .promise() calls are mostly superfluous. Drop them.

function openAllLeves () {
    openAll = 1;
    openLevels().then(function() {
        openAll = 0
    });
}

function openLevels() {
    var promises = [];     
    $('.myClass:not([data-type="T"])').each(function () { // using `map` would be even better
        promises.push(loadLine($(this)));
    });
    return $.when.apply($, promises);
}

function loadLine(thisObj) {;
    switch(nivel) {
        case 1:
            return getPT($(thisObj).attr('data-a'))
        case 2:
            // ...
    }
}

function getPT(psn) {
    var payload = { /* parameters... */ };
    return $.ajax({
        url: webmethod,
        data: payload,
        type: "POST",
        contentType: "application/json; charset=utf-8",
        dataType: "json",
        timeout: 10000,
    }).then(function (data) {
        return paintPT(data);
    }).then(function () { 
        if (openAll) 
           openLevels();
    });
}

Btw, you will probably want to chain the if (openAll) openLevels(); to the return value of openLevels(), not to each single request promise.

MKP · Answer 3 · 2016-08-05T09:25:11.863

0

Thank you both for your responses. I'm working on this changes. This way, I understand that .then() only waits for the promise when a function is passed. So the right way to resolve a promise in .then() would be..

.then(function() {
  defer.resolve();
})

¿?

edited Aug 05 '16 at 09:25

answered Aug 05 '16 at 06:17

MKP

117
3
14

yes, but again, as Bergi pointed out you should avoid using this pattern, promises can be chained together just by returning a new promise in the then callback. – Tiago Engel Aug 05 '16 at 11:21
Also, avoid asking questions in a response, use the comments section for that – Tiago Engel Aug 05 '16 at 11:24

Promises not resolved in a .each() loop

3 Answers3

Avoid the deferred antipattern!