Skip to content

Commit

Permalink
The world isn't ready for moving the Ajax methods to jQuery.ajax.*. H…
Browse files Browse the repository at this point in the history
…ope to move them there some day. Fixes #7146.
  • Loading branch information
jeresig committed Oct 11, 2010
1 parent 1df5084 commit 6245ecb
Showing 1 changed file with 27 additions and 28 deletions.
55 changes: 27 additions & 28 deletions src/ajax.js
Expand Up @@ -240,8 +240,8 @@ jQuery.extend({

window[ jsonp ] = function( tmp ) {
data = tmp;
jQuery.ajax.handleSuccess( s, xhr, status, data );
jQuery.ajax.handleComplete( s, xhr, status, data );
jQuery.handleSuccess( s, xhr, status, data );

This comment has been minimized.

Copy link
@vitorhsb

vitorhsb Oct 12, 2010

Is it possible to create a reference from jQuery.ajax.handleSuccess to jQuery.handleSuccess? This way you could evangelize the use of the new method instead of the old in the documentation, preparing for future releases.

jQuery.handleComplete( s, xhr, status, data );

if ( jQuery.isFunction( customJsonp ) ) {
customJsonp( tmp );
Expand Down Expand Up @@ -281,7 +281,7 @@ jQuery.extend({
}

// Watch for a new set of requests
if ( s.global && jQuery.ajax.active++ === 0 ) {
if ( s.global && jQuery.active++ === 0 ) {
jQuery.event.trigger( "ajaxStart" );
}

Expand All @@ -308,8 +308,8 @@ jQuery.extend({
if ( !done && (!this.readyState ||
this.readyState === "loaded" || this.readyState === "complete") ) {
done = true;
jQuery.ajax.handleSuccess( s, xhr, status, data );
jQuery.ajax.handleComplete( s, xhr, status, data );
jQuery.handleSuccess( s, xhr, status, data );
jQuery.handleComplete( s, xhr, status, data );

// Handle memory leak in IE
script.onload = script.onreadystatechange = null;
Expand Down Expand Up @@ -358,8 +358,8 @@ jQuery.extend({
xhr.setRequestHeader("If-Modified-Since", jQuery.lastModified[s.url]);
}

if ( jQuery.ajax.etag[s.url] ) {
xhr.setRequestHeader("If-None-Match", jQuery.ajax.etag[s.url]);
if ( jQuery.etag[s.url] ) {
xhr.setRequestHeader("If-None-Match", jQuery.etag[s.url]);
}
}

Expand All @@ -378,7 +378,7 @@ jQuery.extend({
// Allow custom headers/mimetypes and early abort
if ( s.beforeSend && s.beforeSend.call(s.context, xhr, s) === false ) {
// Handle the global AJAX counter
if ( s.global && jQuery.ajax.active-- === 1 ) {
if ( s.global && jQuery.active-- === 1 ) {
jQuery.event.trigger( "ajaxStop" );
}

Expand All @@ -388,7 +388,7 @@ jQuery.extend({
}

if ( s.global ) {
jQuery.ajax.triggerGlobal( s, "ajaxSend", [xhr, s] );
jQuery.triggerGlobal( s, "ajaxSend", [xhr, s] );
}

// Wait for a response to come back
Expand All @@ -398,7 +398,7 @@ jQuery.extend({
// Opera doesn't call onreadystatechange before this point
// so we simulate the call
if ( !requestDone ) {
jQuery.ajax.handleComplete( s, xhr, status, data );
jQuery.handleComplete( s, xhr, status, data );
}

requestDone = true;
Expand All @@ -413,9 +413,9 @@ jQuery.extend({

status = isTimeout === "timeout" ?
"timeout" :
!jQuery.ajax.httpSuccess( xhr ) ?
!jQuery.httpSuccess( xhr ) ?
"error" :
s.ifModified && jQuery.ajax.httpNotModified( xhr, s.url ) ?
s.ifModified && jQuery.httpNotModified( xhr, s.url ) ?
"notmodified" :
"success";

Expand All @@ -425,7 +425,7 @@ jQuery.extend({
// Watch for, and catch, XML document parse errors
try {
// process the data (runs the xml through httpData regardless of callback)
data = jQuery.ajax.httpData( xhr, s.dataType, s );
data = jQuery.httpData( xhr, s.dataType, s );
} catch( parserError ) {
status = "parsererror";
errMsg = parserError;
Expand All @@ -436,15 +436,15 @@ jQuery.extend({
if ( status === "success" || status === "notmodified" ) {
// JSONP handles its own success callback
if ( !jsonp ) {
jQuery.ajax.handleSuccess( s, xhr, status, data );
jQuery.handleSuccess( s, xhr, status, data );
}
} else {
jQuery.ajax.handleError( s, xhr, status, errMsg );
jQuery.handleError( s, xhr, status, errMsg );
}

// Fire the complete handlers
if ( !jsonp ) {
jQuery.ajax.handleComplete( s, xhr, status, data );
jQuery.handleComplete( s, xhr, status, data );
}

if ( isTimeout === "timeout" ) {
Expand Down Expand Up @@ -488,10 +488,10 @@ jQuery.extend({
xhr.send( noContent || s.data == null ? null : s.data );

} catch( sendError ) {
jQuery.ajax.handleError( s, xhr, null, sendError );
jQuery.handleError( s, xhr, null, sendError );

// Fire the complete handlers
jQuery.ajax.handleComplete( s, xhr, status, data );
jQuery.handleComplete( s, xhr, status, data );
}

// firefox 1.5 doesn't fire statechange for sync requests
Expand Down Expand Up @@ -574,7 +574,9 @@ function buildParams( prefix, obj, traditional, add ) {
}
}

jQuery.extend( jQuery.ajax, {
// This is still on the jQuery object... for now
// Want to move this to jQuery.ajax some day
jQuery.extend({

// Counter for holding the number of active queries
active: 0,
Expand All @@ -591,7 +593,7 @@ jQuery.extend( jQuery.ajax, {

// Fire the global callback
if ( s.global ) {
jQuery.ajax.triggerGlobal( s, "ajaxError", [xhr, s, e] );
jQuery.triggerGlobal( s, "ajaxError", [xhr, s, e] );
}
},

Expand All @@ -603,7 +605,7 @@ jQuery.extend( jQuery.ajax, {

// Fire the global callback
if ( s.global ) {
jQuery.ajax.triggerGlobal( s, "ajaxSuccess", [xhr, s] );
jQuery.triggerGlobal( s, "ajaxSuccess", [xhr, s] );
}
},

Expand All @@ -615,11 +617,11 @@ jQuery.extend( jQuery.ajax, {

// The request was completed
if ( s.global ) {
jQuery.ajax.triggerGlobal( s, "ajaxComplete", [xhr, s] );
jQuery.triggerGlobal( s, "ajaxComplete", [xhr, s] );
}

// Handle the global AJAX counter
if ( s.global && jQuery.ajax.active-- === 1 ) {
if ( s.global && jQuery.active-- === 1 ) {
jQuery.event.trigger( "ajaxStop" );
}
},
Expand All @@ -646,11 +648,11 @@ jQuery.extend( jQuery.ajax, {
etag = xhr.getResponseHeader("Etag");

if ( lastModified ) {
jQuery.ajax.lastModified[url] = lastModified;
jQuery.lastModified[url] = lastModified;
}

if ( etag ) {
jQuery.ajax.etag[url] = etag;
jQuery.etag[url] = etag;
}

return xhr.status === 304;
Expand Down Expand Up @@ -712,7 +714,4 @@ if ( window.ActiveXObject ) {
// Does this browser support XHR requests?
jQuery.support.ajax = !!jQuery.ajaxSettings.xhr();

// For backwards compatibility
jQuery.extend( jQuery.ajax );

})( jQuery );

7 comments on commit 6245ecb

@jaubourg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only question is: why? Are some people toying with the active counter by hand?

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently - yes. Also some plugins were overwriting $.ajax with another function - causing them to break (as they didn't have the necessary functions any more). We can try to push for this in jQuery 1.5 - and use the 1.4.3 release as an opportunity to warn people.

@jaubourg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign me on this. Maybe the bigger issue is to sensibilize jQuery devs about not shaking the internals of the lib every chance they get ;)

@leeoniya
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know malsup's Taconite plugin replaces jQuery.httpData. maybe that's why it stopped working when i tested 1.4.3pre a few months ago :\

@gilmoreorless
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm all for this change, quick question though: for a plugin that is specifically designed to override $.ajax (eg. http://github.com/appendto/jquery-mockjax), would the better process be something like this?

var _oldAjax = $.ajax;
$.ajax = function () { ...do stuff... };
$.extend($.ajax, _oldAjax);

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gilmoreorless: Yeah, that'd be a good start. Additionally it'd be good to not assume anything about the 'this' inside those methods (it seems like a few methods were doing this.triggerGlobal (a method not found on jQuery.ajax - but found on jQuery itself).

@jaubourg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I was really tempted to override $.ajax back when I first worked on jQuery-jsonp and that's what I did at first but I then decided to create a new method altogether. The rationale was as follows:

  • replacing an existing method meant it would have been exposed to any change to the contract and internals throughout new jQuery releases,
  • replacing $.ajax with $.jsonp in user code is quite a simple process as long as the method's contract is close to the original one as of current jQuery version.

Now, the downside is that your code doesn't get executed by the other ajax-related methods (get, post, etc). Yet, these other methods are just syntactic sugar after all and I think it's a small price to pay in order to avoid potential conflicts everytime something is moved inside non-documented portions of jQuery.

As for taconite, it's another matter entirely seeing as it's plugging itself into an architecture that is not meant to be pluggable (ie. overriding the undocumented httpData). jQuery-mockjax is yet another type of beast that is doomed to depend on changes in contract just like any other dev tool (like jQuery-lint http://james.padolsey.com/javascript/jquery-lint/ ).

My opinion is, while all those plugins are cool and I see no reason for their authors not to go with them, it could be a good idea to inform the jQuery team of any difficulty regarding implementing them "cleanly" so that changes can occur in the internals of the library to help plugin developers. Be it with a forum post or a pull request.

I'm personally very interested in such demands seeing as I'm currently working on a pluggable architecture for ajax but it is clear this goes beyond jQuery.ajax and concerns most of jQuery's methods.

Sorry for the wall of text ;)

Please sign in to comment.