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

Strange behavior of Deferred.then chain in jQuery 3.3.1 #4040

Closed
furyutei opened this Issue Apr 16, 2018 · 17 comments

Comments

Projects
None yet
3 participants
@furyutei

furyutei commented Apr 16, 2018

While using jQuery v3.3.1
I executed the following script on the console of Google Chrome 65.0.3325.181.

var $rejected = $.Deferred().reject( '** rejected **' ).promise(),
    $resolved = $.Deferred().resolve( $rejected, 'resolved' ).promise();

/* case A */
$resolved
    .done( function () { console.log( 'A.done()', arguments ); } ) // doneHandler called
    .fail( function () { console.error( 'A.fail()', arguments ); } );

/* case B */
$.Deferred().resolve()
    .then( function () {
        return $resolved;
    } )
    .done( function () { console.log( 'B.done()', arguments ); } )
    .fail( function () { console.error( 'B.fail()', arguments ); } ); // failHandler called !? Why ?

In case B, I thought that doneHandler would be called like in case A, but in fact it was called failHandler with an argument '** rejected **'.

@dmethvin

This comment has been minimized.

Member

dmethvin commented Apr 17, 2018

Would you agree that this is the same behavior in native Promise?
http://jsbin.com/mepayuj/edit?html,js,console

var rejected = Promise.reject( '** rejected **' ),
    resolved = Promise.resolve( rejected );

Promise.resolve()
    .then( function () {
        return resolved;
    } )
    .then( function () { console.log( 'B.done()', arguments ); } )
    .catch( function () { console.error( 'B.fail()', arguments ); } ); 
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

Well, it seems to be a strange behavior for me, but at least it is unified in cases A and B (both are failed).

var rejected = Promise.reject( '** rejected **' ),
    resolved = Promise.resolve( rejected, 'resolved' );

/* Case A */
resolved
  .then( function () { console.log( 'A.done(): ' + typeof arguments[ 0 ] ); } )
  .catch( function () { console.error( 'A.fail(): ' + typeof arguments[ 0 ] ) ; } ); // failHandler called

/* Case B */
Promise.resolve()
    .then( function () {
        return resolved;
    } )
    .then( function () { console.log( 'B.done(): ' + typeof arguments[ 0 ] ); } )
    .catch( function () { console.error( 'B.fail(): ' + typeof arguments[ 0 ] ); } ); // failHandler called

On the other hand, when jQuery 3.3.1(or jQuery git) is used, it will behave differently in case A and case B (A is done, but B is failed).

var $rejected = $.Deferred().reject( '** rejected **' ).promise(),
    $resolved = $.Deferred().resolve( $rejected, 'resolved' ).promise();

/* case A */
$resolved
    .done( function () { console.log( 'A.done(): ' + typeof arguments[ 0 ] ); } )
    .fail( function () { console.error( 'A.fail(): ' + typeof arguments[ 0 ] ); } );

/* case B */
$.Deferred().resolve()
    .then( function () {
        return $resolved;
    } )
    .done( function () { console.log( 'B.done(): ' + typeof arguments[ 0 ] ); } )
    .fail( function () { console.error( 'B.fail(): ' + typeof arguments[ 0 ] ); } );

Should the behavior be unified in jQuery, too?

In addition to, when jQuery 2.2.4 or jQuery 1.12.4 is used, the behavior is unified (both A and B will be done).

@dmethvin

This comment has been minimized.

Member

dmethvin commented Apr 18, 2018

Note that there is no equivalent to .done() and .fail() in the Promise/A spec, they run synchronously and don't swallow errors so you should expect them to behave differently. The .then() and .catch() in jQuery 3 behave like the Promise/A spec.

@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

I misunderstood the specification of native Promise.
jQuery's deferred.resolve can take multiple arguments, but only one argument with Promise.resolve().

Therefore, for equivalent processing in native Promise, I think that it is necessary to do as follows(both A and B will be done).

var rejected = Promise.reject( [ '** rejected **' ] ),
    resolved = Promise.resolve( [ rejected, 'resolved' ] );

/* Case A */
resolved
    .then( ( arguments ) => { console.log( 'A.done(): ' + typeof arguments[ 0 ] ); } )
    .catch( ( arguments ) => { console.error( 'A.fail(): ' + typeof arguments[ 0 ] ); } );

/* Case B */
Promise.resolve()
    .then( function () {
        return resolved;
    } )
    .then( ( arguments ) => { console.log( 'B.done(): ' + typeof arguments[ 0 ] ); } )
    .catch( ( arguments ) => { console.error( 'B.fail(): ' + typeof arguments[ 0 ] ); } );
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

The argument of native Promise.resolve has a special specification,

[[Resolve]](promise, x). If x is a thenable, it attempts to make promise adopt the state of x
Promises/A+

If the value is a thenable (i.e. has a "then" method), the returned promise will "follow" that thenable, adopting its eventual state;
Promise.resolve() - JavaScript | MDN

but I think that arguments of jQuery's deferred.resolve does not have such a specification.

@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

As a supplement, this behavior only occurs when rejected promise is passed as the first argument to deferred.resolve() (in cases A and B, A will be done, but B will be failed).
It does not occur when the first argument and the second argument are exchanged (in cases A' and B', both will be done).

http://jsbin.com/gecelazuba/edit?html,js,console

var $rejected = $.Deferred().reject( '** rejected **' ).promise(),
    $resolved = $.Deferred().resolve( $rejected, 'resolved' ).promise(),
    $resolved_prime = $.Deferred().resolve( 'resolved', $rejected ).promise();

/* case A */
$resolved
    .done( function () { console.log( "A.done(): " + arguments.length ); } )
    .fail( function () { console.error( "A.fail(): " + arguments.length ); } );

/* case B */
$.Deferred().resolve()
    .then( function () {
        return $resolved;
    } )
    .done( function () { console.log( "B.done(): " + arguments.length ); } )
    .fail( function () { console.error( "B.fail(): " + arguments.length ); } );

/* case A' */
$resolved_prime
    .done( function () { console.log( "A'.done(): " + arguments.length ); } )
    .fail( function () { console.error( "A'.fail(): " + arguments.length ); } );

/* case B' */
$.Deferred().resolve()
    .then( function () {
        return $resolved_prime;
    } )
    .done( function () { console.log( "B'.done(): " + arguments.length ); } )
    .fail( function () { console.error( "B'.fail(): " + arguments.length ); } );
@dmethvin

This comment has been minimized.

Member

dmethvin commented Apr 18, 2018

Sorry but as the examples get longer and more complex it is getting harder for me to understand if there is a problem being reported. Part of my confusion results from giving the name $resolved to a Deferrred that is rejected.

It seems to me that it comes down to this behavior, which is consistent with the standard. Resolving Promise1 using some other rejected Promise2 also rejects Promise1.

Promise.resolve(Promise.reject("sorry"))
 .then(() => console.log("ok"))
 .catch((e) => console.log("fail " + e))  // "fail - sorry"
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

Yes, it is consistent with the standard(because resolve's argument is a thenable, then "follow" that thenable).

// native Promise
Promise.resolve( Promise.reject( "sorry") )
    .then( () => console.log( "ok" ) )
    .catch( ( e ) => console.log( "fail -" + e ) ); // "fail - sorry"

But in jQuery,

// jQuery
$.Deferred().resolve( $.Deferred().reject( "sorry" ) )
    .done( () => console.log( "ok" ) ) // "ok"
    .fail( ( e ) => console.log( "fail -" + e ) );

If it is supposed to be the same as Promise/A spec, "fail - sorry" is expected.
But actually it will be "ok".

@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

Also, if I want to pass more than one argument, I can write like the following code in jQuery.

// jQuery
$.Deferred().resolve( "1", "2", "3" )
    .done( function () { console.log( arguments ); } ); // Arguments(3) ["1", "2", "3"]

However, native Promise got only one argument, so the following code is wrong.

// native Promise
Promise.resolve( "1", "2", "3" )
    .then( function () { console.log( arguments ); } ); // Arguments ["1"]

Therefore, to make it similar to jQuery, it seems that the following code is appropriate.

// native Promise
Promise.resolve( [ "1", "2", "3" ] )
    .then( ( arguments ) => { console.log( arguments ); } ); // (3) ["1", "2", "3"]
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

Then in this format, the following code will be "ok", same as jQuery.

// native Promise
Promise.resolve( [ Promise.reject( "sorry") ] )
    .then( ( arguments ) => console.log( "ok", arguments ) ) // "ok [Promise]"
    .catch( ( e ) => console.log( "fail -" + e ) );

// jQuery
$.Deferred().resolve( $.Deferred().reject( "sorry" ) )
    .done( function () { console.log( "ok", arguments ); } ) // "ok Arguments [Deferred.promise]"
    .fail( ( e ) => console.log( "fail -" + e ) );
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

However, this time, if you try to use then chain as below, there will be a difference between native promise and jQuery.
("ok" in native Promise, but "fail - sorry" in jQuery)

// native Promise
Promise.resolve()
    .then( () => {
        return Promise.resolve( [ Promise.reject( "sorry") ] );
    } )
    .then( ( arguments ) => console.log( "ok", arguments ) ) // "ok [Promise]"
    .catch( ( e ) => console.log( "fail -" + e ) );

// jQuery
$.Deferred().resolve()
    .then( () => {
        return $.Deferred().resolve( $.Deferred().reject( "sorry" ) );
    } )
    .done( function () { console.log( "ok", arguments ); } )
    .fail( ( e ) => console.log( "fail -" + e ) ); // "fail - sorry"
@furyutei

This comment has been minimized.

furyutei commented Apr 18, 2018

Finally, if you set the first argument of resolve to something other than Deferred.promise as below, "ok" is returned this time.

// jQuery
$.Deferred().resolve()
    .then( () => {
        return $.Deferred().resolve( "TEST", $.Deferred().reject( "sorry" ) );
    } )
    .done( function () { console.log( "ok", arguments ); } ) // "ok Arguments(2) ["TEST", Deferred.promise]"
    .fail( ( e ) => console.log( "fail -" + e ) );

Probably, it seems that a special interpretation is done only for the first argument.

@dmethvin

This comment has been minimized.

Member

dmethvin commented Apr 19, 2018

Standard Promise/A only uses one argument. Since jQuery.Deferred allowed more, we still provide that ability.

Are you reporting a bug here? If not I'll close the ticket. If you feel that the documentation needs to be improved you can make suggestions at https://github.com/jquery/api.jquery.com/ . The request for docs changes need to be specific, especially given that these are relatively obscure behaviors.

@furyutei

This comment has been minimized.

furyutei commented Apr 19, 2018

Yes, I am reporting a bug from the beginning.

I think, it should work as follows.

$.Deferred().resolve()
    .then( () => {
        return $.Deferred().resolve( $.Deferred().reject( "sorry" ), "TEST" );
    } )
    .done( function () { console.log( "ok", arguments ); } ) // should be "ok Arguments(2) [Deferred.promise, "TEST"]" !!!
    .fail( ( e ) => console.log( "fail -" + e ) );

If you insist on compatibility with native Promise(like that), I think you should extend the specification of deferred.resolveWith that only uses one argument(args), like Promise/A.

Currently it is as follows,

deferred.resolveWith( context [, args ] )
:
args
Type: Array
An optional array of arguments that are passed to the doneCallbacks.
deferred.resolveWith()

How about extending the specification as follows?

args
Type: Array or thenable(i.e. deferred.promise)

@furyutei

This comment has been minimized.

furyutei commented Apr 19, 2018

My question is simple as below.

$.Deferred().resolve( $.Deferred().reject( "sorry" ), "TEST" )
    .then( function () { console.log( "ok", arguments ); } ) // "ok Arguments(2) [Deferred.promise, "TEST"]"
    .catch( ( e ) => console.log( "fail -" + e ) );

$.Deferred().resolve()
    .then( () => $.Deferred().resolve( $.Deferred().reject( "sorry" ), "TEST" ) )
    .then( function () { console.log( "ok", arguments ); } ) // It should be "ok Arguments(2) [Deferred.promise, "TEST"]" !!!
    .catch( ( e ) => console.log( "fail -" + e ) ); // In fact, "fail -sorry". Why ? It is a bug, isnt it?
@dmethvin

This comment has been minimized.

Member

dmethvin commented Apr 20, 2018

OK, that is a good small case and I agree it seems inconsistent with Promise.
http://jsbin.com/gobolin/1/edit?js,console

@gibson042 what are your thoughts on this? I'm surprised there wouldn't be a Promise/A+ test for it if it's intended behavior.

@gibson042

This comment has been minimized.

Member

gibson042 commented Apr 30, 2018

These examples highlight differences between the then method (including its catch shorthand), whose behavior is specified by Promises/A+ and ECMAScript Promise objects, and the fundamental model of jQuery Deferred objects, which predate both. In terms of ECMAScript vocabulary, jQuery Deferred objects can be fulfilled with thenable objects (i.e., objects with a then method), whereas Promise objects cannot be because resolution is required to enqueue invocation of a then method on the inbound value (with the observable effect being that standard Promise resolution can never supply a thenable to a then onFulfilled callback).

However, the jQuery Deferred behavior is intended for backwards compatibility, and surprising because you're mixing models.

$.Deferred().resolve( $.Deferred().reject( "sorry" ), "TEST" )
    .then( function () { console.log( "ok", arguments ); } ) // "ok Arguments(2) [Deferred.promise, "TEST"]"
    .catch( ( e ) => console.log( "fail -" + e ) );

Deferred#then callbacks receive all settlement values, which in this case means the "done" (onFulfilled) callback gets a rejected Deferred object and a string (and such multiple settlement values are only possible via Deferred#resolve). The callback has no explicit return, so the Deferred returned by .then(…) will be fulfilled with undefined and therefore not invoke a catch callback.

$.Deferred().resolve()
    .then( () => $.Deferred().resolve( $.Deferred().reject( "sorry" ), "TEST" ) )
    .then( function () { console.log( "ok", arguments ); } ) // It should be "ok Arguments(2) [Deferred.promise, "TEST"]" !!!
    .catch( ( e ) => console.log( "fail -" + e ) ); // In fact, "fail -sorry". Why ? It is a bug, isnt it?

The first .then(…) invokes its onFulfilled callback and returns a Deferred resolved with the result (in this case a Deferred thenable, which is unwrapped as part of then handling to reveal a rejected thenable which results in the .then(…) return value also being in a rejected state and therefore ignoring onFulfilled callbacks). When introducing then compatibility, we could have instead done the unwrapping only on the input side, but that would have made the implementation messier and less backwards compatible (i.e., unable to receive multiple settlement values) for the benefit only of such nonstandard cases, a path which we opted to avoid in favor of having then force its output to be interpreted in the context of standard behavior.

@gibson042 gibson042 closed this Apr 30, 2018

@gibson042 gibson042 added the Deferred label Apr 30, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Oct 27, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.