Are you saying there is a bug in that it's not working as documented? Can you point to the documentation that says the .complete() method on a jQXHR should be passed the data as its first argument?

Replying to [comment:1 dmethvin]:

Are you saying there is a bug in that it's not working as documented? Can you point to the documentation that says the .complete() method on a jQXHR should be passed the data as its first argument?

In case of a success, it should, it is unit tested afaik. What I'd like to see is an actual test case.

Also, to the OP, why don't you use .done rather than .complete?

@dmethvin, scrap that, I mixed things with .always :/

Actually, this hit me in real code as well. I'd be for passing the parsed JSON to complete. I ended up re-parsing. Btw, I don't think the OP is saying the docs aren't clear, just that the current behavior is neither intuitive or efficient(since the JSON has already been parsed and correctly passed to success) in terms of dealing with JSON responses in the complete callback. I gotta go with the OP on this one.

Well, now that I think more about the implementation, we may not be able to guarantee the JSON would always be there since the complete callback is also called on failure (when there may not be any JSON to parse). So to make parameters consistent, it may have to stay inefficient, amd admittedly confusing.

Still, the data has been parsed so it would be reasonable to assume the parsed data would be available when you hit always. We _could_ perhaps tack on data as a 3rd argument when available and leave it up to the user to validate callback arguments. I'd be down for that, but if arguments are not validated and JSON is expected, complete callbacks will throw an error whenever there is a failure that does not provide JSON. In my opinion, a reasonable risk taken for the sake of convenience and overall performance, but others may not agree.

There are a few things going on here, all tangled into one "bug report" that isn't a bug but more a series of documented but unfortunate coincidences. I think this is all working as documented:

1) The .complete() method of jQXHR is deprecated, and the .always() method of $.Deferred takes its place.

2) The creator of a $.Deferred() gets the opportunity to define what arguments are passed to .done() and .fail() handlers. They do not have to be identical and in fact often are not.

3) As a result of (2) the .always() handler cannot be guaranteed it will get a consistent set of arguments. The $.Deferred() creator must ensure it by design.

4) To simplify migration from the old-style callbacks and because there was no good reason to change it, we used the same argument lists for the .done() and .fail() methods in 1.5. At this point the trap was set.

5) In 1.6 we added the .always() method, whose added handlers are always called and passed the arg list of the .resolve() or .reject() call. So we get this signature:

jqXHR.always(function(data|jqXHR, textStatus, jqXHR|errorThrown) { });

6) The .always() method isn't guaranteed to have the converted data, either the HTTP request or data conversion may have failed. But if it succeeded, the data should be the first argument. You will need to figure out whether it is the data you wanted or a jQXHR by looking at the third arg to see if it is a string.

7) Yeah.

Error case: http://jsfiddle.net/dmethvin/WVrxw/1/

Success case: http://jsfiddle.net/dmethvin/WVrxw/2/

We're over-thinking this... a lot. The OP clearly should use .done because his intention is to get the parsed data that is only present in case of a success. Using .complete is bad practice here, plain and simple.

Now, we discussed the order of arguments for success and error handlers ad nauseum before: we just cannot change them without breaking every single app in the known universe.

Also, as a side note, you can make sure the json data is on the jqXHR instance by doing so:

$.ajaxSetup({
  responseFields: {
    "json": "responseJSON"
  }
});

Now, everytime you request json successfully, the jqXHR instance will have the responseJSON field set properly.

Also, if we finally merge PR1164, this is fixed because the responseJSON field is added to the jqXHR instance there and it will be there for the error case if the data is proper json to boot.

I'm not sure why I was assuming the old complete was just an alias for always. That wouldn't work anyway, considering the difference in arguments. Thank you jaubuorg for making it clear yet again that you have it covered. I need to read all the ajax docs again. Forgive my ignorance.

As a side note, I'm in favor of landing 1164.

Replying to [comment:8 timmywil]:

I'm not sure why I was assuming the old complete was just an alias for always. That wouldn't work anyway, considering the difference in arguments. Thank you jaubuorg for making it clear yet again that you have it covered. I need to read all the ajax docs again. Forgive my ignorance.

Forgive the docs... I don't remember if responseFields is even mentionned. Also, Dave did a tremendous job at summarizing the madness we got ourselves into with those signatures ;)

As a side note, I'm in favor of landing 1164.

Yeah, I really gotta get to it. It's just that I'm switching tasks like crazy these days.

Just shows how confusing $.Deferred can be. There are like 4 or 5 ways to get a "done" signal (complete, always, done, then, and probably more), and they're all ever so slightly different. I will try to remember to stop using complete. It's not (no longer?) in the docs, so I guess that's a good thing.

Still it's weird that the parsed JSON is nowhere to be found in this function. I will stick to that conclusion, no matter how deprecated complete is.