Skip to content
Permalink
Browse files
Simplified the way a Promise is tested for (removed promiseMarker). R…
…emoved isCancelled helper method from _Deferred, wasn't used nor tested. Reworked jQuery.Deferred and removed unnecessary variables. Also ensured a Promise will return itself when asked for a Promise. Finally, the jQuery.when tests have been revamped.
  • Loading branch information
jaubourg committed Dec 30, 2010
1 parent 64902e0 commit 7b1e873
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 44 deletions.
@@ -75,10 +75,7 @@ var jQuery = function( selector, context ) {
indexOf = Array.prototype.indexOf,

// [[Class]] -> type pairs
class2type = {},

// Marker for deferred
promiseMarker = [];
class2type = {};

jQuery.fn = jQuery.prototype = {
init: function( selector, context ) {
@@ -874,7 +871,7 @@ jQuery.extend({

// resolve with this as context and given arguments
resolve: function() {
deferred.fire( this , arguments );
deferred.fire( jQuery.isFunction( this.promise ) ? this.promise() : this , arguments );
return this;
},

@@ -888,11 +885,6 @@ jQuery.extend({
cancelled = 1;
callbacks = [];
return this;
},

// Has this deferred been cancelled?
isCancelled: function() {
return !!cancelled;
}
};

@@ -903,38 +895,36 @@ jQuery.extend({
// Typical success/error system
Deferred: function( func ) {

var errorDeferred = jQuery._Deferred(),
deferred = jQuery._Deferred(),
successCancel = deferred.cancel;
var deferred = jQuery._Deferred(),
failDeferred = jQuery._Deferred();

// Add errorDeferred methods and redefine cancel
jQuery.extend( deferred , {

fail: errorDeferred.then,
fireReject: errorDeferred.fire,
reject: errorDeferred.resolve,
isRejected: errorDeferred.isResolved,
fail: failDeferred.then,

This comment has been minimized.

Copy link
@rmurphey

rmurphey Dec 30, 2010

Was just reading http://wiki.commonjs.org/wiki/Promises/A and wanted to raise the question of whether jQuery's Deferred ought to follow it more closely regarding passing both success and failure functions to then(), rather than passing success to then() and failure to fail(). I don't know that there are enough reference implementations to be right or wrong here, but I do think it's worth considering whether to stray from an emerging API.

This comment has been minimized.

Copy link
@jaubourg

jaubourg Dec 31, 2010

Author Member

To add an error callback you'd have to do then( null?, errorCallback ). I dunno for you, but I find that a little convoluted. Anyway, "fail" seems more natural to me and I think it should also be to jQuery users who are already used to separate options for success and error callbacks in ajax for instance.

fireReject: failDeferred.fire,
reject: failDeferred.resolve,
isRejected: failDeferred.isResolved,
// Get a promise for this deferred
// If obj is provided, the promise aspect is added to the object
promise: function( obj ) {
obj = obj || {};
for ( var i in { then:1 , fail:1 , isResolved:1 , isRejected:1 , promise:1 } ) {
obj[ i ] = deferred[ i ];
}
jQuery.each( "then fail isResolved isRejected".split( " " ) , function( _ , method ) {
obj[ method ] = deferred[ method ];
});
obj.promise = function() {
return obj;
};
return obj;
}

} );

// Remove cancel related
delete deferred.cancel;
delete deferred.isCancelled;

// Add promise marker
deferred.promise._ = promiseMarker;

// Make sure only one callback list will be used
deferred.then( errorDeferred.cancel ).fail( successCancel );
deferred.then( failDeferred.cancel ).fail( deferred.cancel );

// Unexpose cancel
delete deferred.cancel;

// Call given func if any
if ( func ) {
@@ -946,7 +936,7 @@ jQuery.extend({

// Deferred helper
when: function( object ) {
object = object && object.promise && object.promise._ === promiseMarker ?
object = object && jQuery.isFunction( object.promise ) ?
object :
jQuery.Deferred().resolve( object );
return object.promise();
@@ -1024,30 +1024,26 @@ test("jQuery.Deferred()", function() {

test("jQuery.when()", function() {

expect( 14 );

var fakeDeferred = { then: function() { return this; } };

fakeDeferred.then._ = [];
expect( 21 );

// Some other objects
jQuery.each( {

"Object with then & no marker": { then: jQuery.noop },
"Object with then & marker": fakeDeferred,
"string 1/2": "",
"string 2/2": "some string",
"number 1/2": 0,
"number 2/2": 1,
"boolean 1/2": true,
"boolean 2/2": false,
"an empty string": "",
"a non-empty string": "some string",
"zero": 0,
"a number other than zero": 1,
"true": true,
"false": false,
"null": null,
"undefined": undefined,
"custom method name not found automagically": {custom: jQuery._Deferred().then}
"a plain object": {}

} , function( message , value ) {

notStrictEqual( jQuery.when( value ) , value , message );
ok( jQuery.isFunction( jQuery.when( value ).then( function( resolveValue ) {
strictEqual( resolveValue , value , "Test the promise was resolved with " + message );
} ).promise ) , "Test " + message + " triggers the creation of a new Promise" );

} );

0 comments on commit 7b1e873

Please sign in to comment.