14

I have three functions i'm trying to run, the first two are doing some async stuff that need data for the third to use. I want the third function to fire only when 1 and 2 are both done. this is the general structure but the final function is firing before 1 and 2 finish.

function run() {
    var data1 = {};
    var data2 = {};

    $.when(first(), second()).done(constructData());

    function first() {
        var d = new $.Deferred();

        //do a bunch of stuff async
        data1 = {};

        d.resolve();
    }
    function second() {


        var d = new $.Deferred();

        //do a bunch of stuff async
        data2 = {};
        d.resolve();
    }
    function constructData() {
        //do stuff with data1 and data2
    }

}

Answer was to not call construct data immediately

 $.when(first(), second()).done(constructData);
Brian
  • 4,328
  • 13
  • 58
  • 103

2 Answers2

26

You should return promise object. You also have an error in this line:

$.when(first(), second()).done(constructData());

it should be

$.when(first(), second()).done(constructData); // don't call constructData immediately

So all together it could be:

function run() {
    var data1 = {};
    var data2 = {};

    $.when(first(), second()).done(constructData);

    function first() {
        return $.Deferred(function() { // <-- see returning Deferred object
            var self = this;

            setTimeout(function() {   // <-- example of some async operation
                data1 = {func: 'first', data: true};
                self.resolve();       // <-- call resolve method once async is done
            }, 2000);
        });
    }
    function second() {
        return $.Deferred(function() {
            var self = this;
            setTimeout(function() {
                data2 = {func: 'second', data: true};
                self.resolve();
            }, 3000);
        });
    }
    function constructData() {
        //do stuff with data1 and data2
        console.log(data1, data2);
    }
}

http://jsfiddle.net/FwXZC/

dfsq
  • 191,768
  • 25
  • 236
  • 258
  • wow, didn't know that function can be written inside deferred object this way, thanks! – Denis Oct 04 '13 at 14:00
  • @Denis I don't think that's the proper way of doing it. According to the jQuery [docs](https://api.jquery.com/jquery.deferred/): The `beforeStart` parameter is _"a function that is called just before the constructor returns."_. So that function is called before the deferred object is created. – Duncan Lukkenaer Dec 30 '16 at 11:56
  • @Duncan Of course `beforeStart` is executed before deferred object is created, so? :D (not sure what is the problem that you saw here). And yes, this is how you can do it, and imho this is the most natural and convenient way of doing it, as it aligned with Promise usage too. – dfsq Dec 30 '16 at 12:11
  • @dfsq It seems strange to me to call a method of an instance that has not been fully constructed yet (`.resolve()` in this case). I'm sure it will work fine, but from an OOP POV I wouldn't encourage this. But of course JavaScript is JavaScript and you can do whatever you want. – Duncan Lukkenaer Dec 30 '16 at 14:57
  • @Duncan Oh, I see your confusion. By the time `beforeStart` is invoked `this` instance has already been constructed (although `$.Deferred` hasn't return it yet), so it has all necessary methods like `resolve`, etc. So of course it is perfectly safe to use deferred like I do in my answer (otherwise, it wouldn't be in jQuery, don't you think so?). Check it if you want confirmation: http://james.padolsey.com/jquery/#v=git&fn=jQuery.Deferred – dfsq Dec 30 '16 at 15:20
  • @dfsq After some research I see that you're right. I found out that the callback of the deferred factory also includes the actual deferred-instance as an argument. So instead of creating a `self` variable like you do in your answer, you can even do something like this, for example: `return $.Deferred(def => def.resolve())`. It works totally fine, I just haven't seen this kind of pattern before, and the argument name `beforeStart` made it more confusing. – Duncan Lukkenaer Jan 03 '17 at 10:03
1

I think you should have first() and second() return a promise: return d.promise();. From the docs:

If a single argument is passed to jQuery.when and it is not a Deferred or a Promise, it will be treated as a resolved Deferred and any doneCallbacks attached will be executed immediately.

I suspect this might be why the when call is calling constructData too soon.

It's hard to tell from you code, but be sure you are calling d.resolve() after the async operations have completed.

You might find that a more natural approach to explicitly setting data1 and data2 is instead to use the data that is supplied when resolve is called. This would mean that your when call would look something like this:

$.when(first(), second()).done(function(result1, result2) {
    data1 = result1[0];
    data2 = result2[0];

    constructData();
});

Note that the exact format of results supplied to the done method depends on the nature of the deferred objects. If the promises are returned from a call to $.ajax, the results should be of the form [data, statusText, jqXhrObject].

nick_w
  • 14,758
  • 3
  • 51
  • 71
  • Yeah I didn't add all of the async code it's all a bunch of custom non jquery ajax requests, error was how I called the done function – Brian Feb 22 '13 at 08:16