Wrapping Jquery $.ajax Behind A Facade In Q.js Without Refactoring

It's possible this should be on code review, but here we go! I have a fairly large application with a lot of ajax calls. I started using Q for some async stuff, and figured I would

Solution 1:

You're overthinking it. Q is designed to interoperate with jQuery promises.

If you want to convert a function to return a Q promise - just wrap it with Q():

myAjaxMethod(); // returns a jQuery promiseQ(myAjaxMethod()); // returns a Q promise that wraps the jQuery one

So for example - your ajax method can be:

functionajax(config){
    returnQ($.ajax(config));
}

Which is shorter and less error prone.

Solution 2:

It seems like for the sake of one good idea (wrap jQuery's promises in Q promises) you've come up with two or three bad ideas. That stuff you're doing with dfd.promise.then is complete insanity, and it doesn't work. Observe:

var p = App.ajax({ url: "..." });

p.then(function (data) {
    // ok, no problemconsole.log(data);
});

// if p is not hopelessly broken at this point, the following statement// should have exactly the same outcome as the previous one
p.then(function (data) {
   // Logs "undefined"! You REPLACED p's .then() method with// one from an empty promise in the last statement!console.log(data);
});

Even if you find a way around the particular issue above, it's not wise to do this sort of thing without having a deep understanding of the implications. I have not read the Q library's source in much detail, but it would not surprise me if there are some internal dependencies that rely on the assumption that the then method isn't getting swapped out for something else.

As Benjamin Gruenbaum says, don't overthink it. Don't try to turn promises into something they're not in an attempt to avoid updating your code. Promises/A+ compliant promises only pass one argument into their .then() handlers. By trying to circumvent that, you're completely undermining the good idea you started off with.

Solution 3:

The best I can offer is not dramatically different from some of the stuff in the question, however is is more economically mechanised and exploits Q's ability to coerce a jQuery promise, as recommended by Benjamin G.

First a utility function :

functionunspread() {
    returnArray.prototype.slice.call(arguments);
}

Now in your ajax() function you can use the unspread() utility to bundle jQuery's multiple args :

functionajax(options) {
     returnQ($.ajax(options).then(unspread, unspread));
}

Success and failure handlers unfortunately have to be dissimilar due to the nature of Q's .spread() method, which quite correctly spreads only on success, not on failure.

functionfoo(options) {
    returnajax(options).spread(function(response, textStatus, xhr) {
        console.dir(response);
        console.log(textStatus);
        console.dir(xhr);
        return response;//or rebundle the args into an array/object
    }, function(arr) {
        console.dir(arr[0]);//xhrconsole.log(arr[1]);//textStatusconsole.dir(arr[2]);//ErrorThrownthrow arr;//or any of its elements
    });
}

If you really wanted named error arguments in Q, then here's a decidedly messy (and untested) approach :

functionfoo(options) {
    returnajax(options).spread(function(response, textStatus, xhr) {
        console.dir(response);
        console.log(textStatus);
        console.dir(xhr);
        return response;
    }, function(err) {
        if($.isArray(err)) {
            //it's a $.ajax error arrayreturn err; //put the error array on the success path so it can be spread
        } else {
            throw err;//maybe it's an unbundeled error
        }
    }).spread(function(xhr, textStatus, ErrorThrown) {
        if(arguments.length == 3) {//test because the genuine success path will lead you here too.console.dir(xhr);
            console.log(textStatus);
            console.dir(ErrorThrown);
        }
    });
}

But, even if you could get that to work, it's rather extreme just to obtain named args.

I'm sure a "Q.superSpread()" method could be written to do the job. I gave it 10 minutes and decided it was not trivial, equally extreme and probably conceptually unsound.