Skip to content
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.

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

Improve when/then for non-ajax usage #2545

Closed
krazyjakee opened this issue Aug 18, 2015 · 9 comments
Closed

Improve when/then for non-ajax usage #2545

krazyjakee opened this issue Aug 18, 2015 · 9 comments

Comments

@krazyjakee
Copy link

The following code is a simple example of then chaining as I would expect it to work. In the documentation here: https://api.jquery.com/jquery.when/ the way $.Deferred is used in the non-ajax example does not explain how I should structure the code below. It also makes the code so complicated and ugly, I may as well use traditional callbacks to clean up. My suggestion is to let "then" handle a javascript promise as expected below.

The result is that both synchronous functions fire instantly instead of waiting for the previous async promises to resolve.

http://jsfiddle.net/0r62we79/

var count = 0;

var functionAsync = function(){
    return new Promise(function(success, fail){
        setTimeout(function(){
            success();
        }, 2000);
    });
}

var functionSync = function(){
    return new Promise(function(success, fail){
        count++;
        $(document.body).html(count);
        success();
    });
}

$.when(functionAsync())
.then(functionSync())
.then(functionAsync())
.then(functionSync())

In addition, if a promise is not returned, the next "then" function should be called immediately. Then I can unwrap the function from it's promise in functionSync.

I and many others are struggling with the bizarre terminology, workflow and ugly code of the when feature. Even if I have misunderstood everything, the documentation needs to be improved.

@mgol
Copy link
Member

mgol commented Aug 18, 2015

Your example is incorrect; then accepts a function as a parameter and you're passing a promise to it. Before the initial async promise is resolved, both sync promises are created so you see 2 printed before any async operation happens.

Also, jQuery 3's Deferred follow the Promises/A+ spec which mandates that then handlers are always invoked asynchronously.

A working example: http://jsfiddle.net/0r62we79/1/.

@mgol mgol closed this as completed Aug 18, 2015
@krazyjakee
Copy link
Author

I've improved the example further and must still argue that the first then function should not be firing at the same time as the initial when function. It seems the second synchronous then function on line 22 functions correctly and waits for the previous async function to complete.

I understand that a then function is invoked asynchronously but it should not be invoked until the previous function in the queue is completed.

Notice it shows 1 straight away instead of waiting for the first async function to complete.
http://jsfiddle.net/0r62we79/2/

EDIT: http://jsfiddle.net/0r62we79/3/

ok, so not passing anything to when and moving right onto then solves the problem. This is very confusing since $.when() returning a resolved promise is redundant. Something is definitely wrong here in terms of usability.

@scottgonzalez
Copy link
Member

The new example is still incorrect. The value passed to $.when() in this example should be the promise returned by functionAsync, not the function itself. By passing the function itself, the function is considered the resolved value, which results in the first .then() being executed without a delay.

@krazyjakee
Copy link
Author

@scottgonzalez @mzgol why in one instance should I return a Promise and in the next I should return a function? Obviously because that's the way it works but why does it work like that? Shouldn't there be consistency in the majority of use cases? My example is not edge case by any means.

In this example I remove jQuery from the equation: http://jsfiddle.net/0r62we79/4/

See how much cleaner the original spec code is? I want the most simple, cross-browser solution possible, is there anything I can change in my example to simplify it with jQuery?

@mgol
Copy link
Member

mgol commented Aug 19, 2015

In this example I remove jQuery from the equation: http://jsfiddle.net/0r62we79/4/

This is not a direct equivalent of the jQuery version, this is: http://jsfiddle.net/0r62we79/5/

Promise.resolve and jQuery.when allow you to take a regular value & wrap it in a promise that resolves to it or a thenable that's changed to a promise resolving to the same value the thenable will. Therefore, a function is nothing special when passed to these functions - it's not a promise but just a regular value, Promise.resolve and jQuery.when wrap it in a promise that will resolve to this function.

Of course, if you know a promise you get is the kind of promise you use in your code, you don't need the conversion step and it's enough to attach .then directly to the promise, hence why you didn't need to use Promise.resolve in your example. But if you get another kind of promise (e.g. a jQuery promise in a code using ES6 promises or an ES6 promise in a code using jQuery ones) you need to convert it first using Promise.resolve or jQuery.when.

On the other hand, .then accepts a success and/or error callback as a parameter and that's what gets invoked after the promise resolves. Promise.resolve and .then are not equivalent in any way so it's not surprising they accept different kind of input.

I'm afraid we're entering Stack Overflow area, though; please search further help here as there's definitely no bug in jQuery reported here. The concepts that you struggle with are required to comprehend to be able to write any promise-based code, not only using jQuery but also using standard promises.

One of the resources you might want to read that should be pretty comprehensive is You Don't Know JS - Chapter 3: Promises.

@mgol
Copy link
Member

mgol commented Aug 19, 2015

To finish - the only thing I see that's currently wrong here is that AFAIK jQuery.when doesn't convert another thenable to a jQuery promise but that should be covered by #2018. In other words, this: http://jsfiddle.net/0r62we79/9/ should print 1 after 2 seconds, not immediately, as this: http://jsfiddle.net/0r62we79/11/ does.

@krazyjakee
Copy link
Author

@mzgol great answers, thank you!

I'll keep an eye on #2018. If that issue is addressing your example here: http://jsfiddle.net/0r62we79/9/ then it will cover the exact problem I was having. The idea that when would not mean when this happens, return a promise seemed very broken/misleading to me (hence my confusion about terminology), but hopefully #2018 will solve that.

Thanks again!

@mgol
Copy link
Member

mgol commented Aug 19, 2015

@krazyjakee OK, #2018 has apparently not covered that issue, #2546 is. So we've discovered a bug in jQuery.when thanks to this issue after all. ;) Thanks for the pointers.

@krazyjakee
Copy link
Author

@mzgol sorry I couldn't be more direct with my descriptions. As you can see, I'm just starting to make use of these new features of javascript.

Thanks again.

@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.
Labels
None yet
Development

No branches or pull requests

3 participants