Skip to content

$.when, $.progress and already resolved deferred #1894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
nicolashenry opened this issue Dec 2, 2014 · 7 comments
Closed

$.when, $.progress and already resolved deferred #1894

nicolashenry opened this issue Dec 2, 2014 · 7 comments
Assignees
@dmethvin
Labels
Deferred
Milestone
1.12.0/2.2.0

Comments

@nicolashenry
Copy link
Contributor

This code :

$.when($.Deferred().notify(1).resolve(), 
$.Deferred().notify(2).resolve(), 
$.Deferred().notify(3).resolve())
.progress(function() {
    console.log(arguments);
});

displays "[1,2,undefined]".
I would expect to have "[1,2,3]" as a result because this behavior with the last notify does not appear to be described in the documentation. If any of the deferred objects is not already resolved, the display is "[1,2,3]".
@dmethvin
Copy link
Member

dmethvin commented Dec 2, 2014

No matter now many (N>1) are combined, the last one never gets a progress notification. http://jsbin.com/muhuxireko/1/edit?js,console

For reference, the docs say:

When the Deferred is resolved or rejected, progress callbacks will no longer be called, with the exception that any progressCallbacks added after the Deferred enters the resolved or rejected state are executed immediately when they are added, using the arguments that were passed to the .notify() or notifyWith() call. http://api.jquery.com/deferred.progress/

When deferred.notify is called, any progressCallbacks added by deferred.then or deferred.progress are called. Callbacks are executed in the order they were added. Each callback is passed the args from the .notify(). Any calls to .notify() after a Deferred is resolved or rejected (or any progressCallbacks added after that) are ignored. http://api.jquery.com/deferred.notify/

Frankly I wouldn't have even expected the combined Deferred to report progress from the individual ones. These kind of situations are probably some of many reasons why standard Promise punted on progress. Others, thoughts?

@dmethvin
Copy link
Member

dmethvin commented Dec 3, 2014

@jaubourg any input?

@jaubourg
Copy link
Member

jaubourg commented Dec 4, 2014

I cannot properly investigate since I'm travelling until monday. I'll look into it then.

@nicolashenry
Copy link
Contributor Author

I think the problem comes from here (deferred.js#L134) :

resolveValues[ i ].promise()
    .done( updateFunc( i, resolveContexts, resolveValues ) )
    .fail( deferred.reject )
    .progress( updateFunc( i, progressContexts, progressValues ) );

The progress update should perhaps be done first :

resolveValues[ i ].promise()
    .progress( updateFunc( i, progressContexts, progressValues ) )
    .done( updateFunc( i, resolveContexts, resolveValues ) )
    .fail( deferred.reject );

@jaubourg
Copy link
Member

jaubourg commented Dec 9, 2014

I think that's the problem too. Wanna make a PR for this with a nice unit test to boot @nicolashenry? :)

@nicolashenry nicolashenry mentioned this issue Dec 9, 2014
Closed
@nicolashenry
Copy link
Contributor Author

Is it okay that way?

@dmethvin dmethvin added this to the 3.0.0 milestone Dec 10, 2014
@dmethvin dmethvin self-assigned this Dec 10, 2014
@markelog
Copy link
Member

Merged! Thank you

@gibson042 gibson042 mentioned this issue Jan 13, 2015
Closed
@homm homm mentioned this issue Jan 14, 2015
Closed
markelog pushed a commit that referenced this issue Nov 10, 2015
@nicolashenry @markelog
Deferred: Fix $.when with resolved deferred and progress callbacks
fb77e6e 
Fixes gh-1894
Closes gh-1915
@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 7, 2016
@cssmagic cssmagic mentioned this issue May 18, 2016
Open
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dmethvin dmethvin

Labels
Deferred
Milestone
1.12.0/2.2.0
Development

Successfully merging a pull request may close this issue.

Deferred: Fix $.when with resolved deferred and progress callbacks
4 participants
@nicolashenry @dmethvin @jaubourg @markelog