Bug Tracker

Opened 7 years ago

Closed 7 years ago

#13841 closed bug (wontfix)

$.when().always gives different types of arguments depending on resolve/rejects

Reported by: karl.westin@… Owned by:
Priority: low Milestone: None
Component: deferred Version: 2.0.0
Keywords: Cc: jaubourg
Blocked by: Blocking:

Description

Consider the following code:

$.when(a, b).always(function() {
  console.log(arguments);
});

if both a and b are resolved, the callback receives two arguments arrays
if a is resolved and b is rejected, we receive the same
if a is rejected (before anything happens to b), the callback receives the arguments a was rejected with. I expected this to be one "arguments" array

Please see the example here: http://jsbin.com/ariquj/3/edit

I understand that we wanna call "always" as soon as possible, but i still think the argument types should be the same. (i.e. always arrays)

Change History (3)

comment:1 Changed 7 years ago by dmethvin

Cc: jaubourg added
Component: unfileddeferred
Priority: undecidedlow

The pattern is a little clearer with three of them: http://jsbin.com/ariquj/4/edit

It definitely seems a challenge to use the data that's returned. Perhaps the maximum size array should be returned with undefined for pending ones?

comment:2 Changed 7 years ago by jaubourg

This is a false dilemma. The real question is : why would you use always if you care about the arguments that much? Surely, the logic in the always handler is not that "common".

You don't even need a contrived example using when to show the "issue". Just take a simple ajax request as an example :

$.ajax( /* ... */ ).always( function( a, b, c ) {
  // So what are a, b and c ? Well it depends, of course!
} );

When you call always on a promise, the arguments depend on the state of the promise and, also, on the internals of whatever code created the promise.

So you can use an ugly anti-pattern and use state to know how to deal with the arguments:

promise.always( function() {
  if ( promise.state() === "resolved" ) {
    // deal with arguments as in a done handler
  } else {
    // deal with arguments as in a fail handler
  }
} );

Of course, looking at the comments in the anti-pattern, one can clearly see what the proper pattern actually is:

promise.done( function() {
  // deal with arguments as in a done handler (duh!)
} ).fail( function() {
  // deal with arguments as in a fail handler (duh!)
} );

Now, if you need to group common logic together, then use always... but only for logic that does not depend on arguments : it wouldn't be common logic anymore otherwise because arguments are not the same when resolved or when rejected. You can rince and repeat as needed :

promise.always( function() {
  // common code before
} ).done( function() {
  // code dependant on resolve values
} ).fail( function() {
  // code depending on rejection values
} ).always( function() {
  // common code after
} );

Realistically, you can have common logic which has parts that depend on the actual state (and associated arguments) of the underlying promise. It would be more readable and maintainable to have it all in one go in a single always handler. You can use then to mitigate the ugliness of the anti-pattern :

promise.then( function() {
  // Some logic
  return { resolved: someValue };
}, function() {
  // Some other logic
  return { rejected: someOtherValue };
} ).always( function( result ) {
  someCommonLogic();
  if ( result.resolved ) {
    showData( result.resolved );
  } else {
    showError( result.rejected );
  }
  someOtherCommonLogic();
  if ( result.rejected ) {
    tryAgainLater();
  }
  someFinalCommonLogic();
} );

It's quite the silly example but you get the idea.

The approach decouples the way you handle (and transform) arguments from the bulk of the actual common logic. In short, it keeps done and fail specifities as separated as desirable.

Now, getting back at when, yes, the arguments when the resulting promise is resolved are different from when the promise is rejected (and so is this). But even if you create an alternate helper that rejects with an array containing undefined everywhere except for the index of the promise that triggered the rejection, you won't solve the issue at hand here: using always, if you wanna deal with the arguments in a meaningful fashion, you need to somehow determine if the resulting promise is resolved or rejected which, most (if not all) of the time, is an anti-pattern.

comment:3 Changed 7 years ago by dmethvin

Resolution: wontfix
Status: newclosed

Okay, I see jaubourg's point here. If you care about the return value then use the .done() or .fail() handler where you know what it's going to return. Save .always() for cases where you want to do universal cleanup regardless of the returned values.

Note: See TracTickets for help on using tickets.