Permalink
Browse files

Preserves context objects when multiple Deferreds are passed to $.whe…

…n(). Context is an array containing the original contexts in order. When non-observable value is given, associated context is undefined. In case only a single non-observable value is given, context is the global object (thanks so much Function.prototype.apply!). Fixes #11749.
  • Loading branch information...
jaubourg committed May 12, 2012
1 parent b6581df commit f93a2f569d31c4d1fc86ff3ae9605309ac491d68
Showing with 36 additions and 18 deletions.
  1. +16 −12 src/deferred.js
  2. +20 −6 test/unit/deferred.js
@@ -97,35 +97,39 @@ jQuery.extend({
var i = 0,
resolveValues = sliceDeferred.call( arguments ),
length = resolveValues.length,
progressValues = new Array( length ),

// the count of uncompleted subordinates
remaining = length !== 1 || ( subordinate && jQuery.isFunction( subordinate.promise ) ) ? length : 0,

// the master Deferred. If resolveValues consist of only a single Deferred, just use that.
deferred = remaining === 1 ? subordinate : jQuery.Deferred(),
promise = deferred.promise(),

// Update function for both resolve and progress values
updateFunc = function( i, arr ) {
updateFunc = function( i, contexts, values ) {
return function( value ) {
arr[ i ] = arguments.length > 1 ? sliceDeferred.call( arguments ) : value;
if( arr === progressValues ) {
deferred.notifyWith( promise, arr );
contexts[ i ] = this;
values[ i ] = arguments.length > 1 ? sliceDeferred.call( arguments ) : value;
if( values === progressValues ) {
deferred.notifyWith( contexts, values );
} else if ( !( --remaining ) ) {
deferred.resolveWith( promise, arr );
deferred.resolveWith( contexts, values );
}
};
};
},

progressValues, progressContexts, resolveContexts;

// add listeners to Deferred subordinates; treat others as resolved
if ( length > 1 ) {
progressValues = new Array( length );
progressContexts = new Array( length );
resolveContexts = new Array( length );
for ( ; i < length; i++ ) {
if ( resolveValues[ i ] && jQuery.isFunction( resolveValues[ i ].promise ) ) {
resolveValues[ i ].promise()
.done( updateFunc( i, resolveValues ) )
.done( updateFunc( i, resolveContexts, resolveValues ) )
.fail( deferred.reject )

This comment has been minimized.

@gibson042

gibson042 May 14, 2012

Member

We were previously providing the same context on resolution and rejection (well, plus or minus a .promise()). Shouldn't we continue to do so? For similar reasons, I'd also like to provide arrays on L141 instead of undefined.

This comment has been minimized.

@jaubourg

jaubourg May 14, 2012

Author Member
  • the context of a rejection cannot be an array since it is the context of the rejection of a single element in the join (exactly like the rejection values btw): we never rejected with the same context as we resolved, ever.

  • returning an array line 141 is a no go, for the exact same reason we're not embedding resolve values in an array in case of a single value:

    $.when( $.when( "hello" ) ).done(function() {
      this == [ [ undefined ] ];
    });

    this kind of recursive embedding is a nightmare to deal with when you have services that make use of $.when internally.

What I find painful though, is that in case of a single value, the context undefined will turn into window thanks to apply.

This comment has been minimized.

@gibson042

gibson042 May 14, 2012

Member
  • I stand embarrassingly corrected
  • I don't follow you. If you move array assignments out of the if, then zero- and one-argument auto-resolve will have as context an uninitialized array of the correct length, which would not be wrapped by a subsequent jQuery.when because of the recycling on L105. It's just unexpected that jQuery.when() and jQuery.when( notAPromise ) resolve with global context but every other signature resolves with array context (excepting jQuery.when( promise ), which is basically a null wrapper).
.progress( updateFunc( i, progressValues ) );
.progress( updateFunc( i, progressContexts, progressValues ) );
} else {
--remaining;
}
@@ -134,10 +138,10 @@ jQuery.extend({

// if we're not waiting on anything, resolve the master
if ( !remaining ) {
deferred.resolveWith( deferred, resolveValues );
deferred.resolveWith( resolveContexts, resolveValues );
}

return promise;
return deferred.promise();
}
});

@@ -289,7 +289,7 @@ test( "jQuery.Deferred.then - context", function() {

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

expect( 23 );
expect( 34 );

// Some other objects
jQuery.each( {
@@ -307,15 +307,23 @@ test( "jQuery.when" , function() {
} , function( message , value ) {

ok( jQuery.isFunction( jQuery.when( value ).done(function( resolveValue ) {
strictEqual( this, window, "Context is the global object with " + message );
strictEqual( resolveValue , value , "Test the promise was resolved with " + message );
}).promise ) , "Test " + message + " triggers the creation of a new Promise" );

} );

ok( jQuery.isFunction( jQuery.when().done(function( resolveValue ) {
strictEqual( resolveValue , undefined , "Test the promise was resolved with no parameter" );
strictEqual( this, window, "Test the promise was resolved with window as its context" );
strictEqual( resolveValue, undefined, "Test the promise was resolved with no parameter" );
}).promise ) , "Test calling when with no parameter triggers the creation of a new Promise" );

var context = {};

jQuery.when( jQuery.Deferred().resolveWith( context ) ).done(function() {
strictEqual( this, context, "when( promise ) propagates context" );
});

var cache, i;

for( i = 1 ; i < 4 ; i++ ) {
@@ -330,7 +338,7 @@ test( "jQuery.when" , function() {

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

expect(53);
expect( 119 );

var deferreds = {
value: 1,
@@ -362,11 +370,15 @@ test("jQuery.when - joined", function() {
shouldNotify = willNotify[ id1 ] || willNotify[ id2 ],
expected = shouldResolve ? [ 1, 1 ] : [ 0, undefined ],
expectedNotify = shouldNotify && [ willNotify[ id1 ], willNotify[ id2 ] ],
code = id1 + "/" + id2;
code = id1 + "/" + id2,
context1 = defer1 && jQuery.isFunction( defer1.promise ) ? defer1 : undefined,
context2 = defer2 && jQuery.isFunction( defer2.promise ) ? defer2 : undefined;

var promise = jQuery.when( defer1, defer2 ).done(function( a, b ) {
jQuery.when( defer1, defer2 ).done(function( a, b ) {
if ( shouldResolve ) {
deepEqual( [ a, b ], expected, code + " => resolve" );
strictEqual( this[ 0 ], context1, code + " => first context OK" );
strictEqual( this[ 1 ], context2, code + " => second context OK" );
} else {
ok( false , code + " => resolve" );
}
@@ -376,8 +388,10 @@ test("jQuery.when - joined", function() {
} else {
ok( false , code + " => reject" );
}
}).progress(function progress( a, b ) {
}).progress(function( a, b ) {
deepEqual( [ a, b ], expectedNotify, code + " => progress" );
strictEqual( this[ 0 ], expectedNotify[ 0 ] ? context1 : undefined, code + " => first context OK" );
strictEqual( this[ 1 ], expectedNotify[ 1 ] ? context2 : undefined, code + " => second context OK" );
});
} );
} );

1 comment on commit f93a2f5

@jaubourg

This comment has been minimized.

Copy link
Member Author

jaubourg commented on f93a2f5 May 12, 2012

@gibson042 you may wanna look at this one. Unit tests are thorough enough so you can try and go wild.

Please sign in to comment.