Bug Tracker

Opened 5 years ago

Closed 5 years ago

#14449 closed feature (notabug)

Allow done/fail callbacks to return deferreds

Reported by: iliakan@… Owned by:
Priority: high Milestone: None
Component: deferred Version: 2.1.0-beta1
Keywords: 2.1-discuss Cc:
Blocked by: Blocking:

Description

As discussed with @dmethvin on jQuery Russia, callbacks assigned to deferreds using .done/fail should be allowed to return deferreds which effectively fit in to the async chain.

Just like .then was fixed and now is the alias to .pipe, .done/fail should be fixed also.

It is not going to break anything, because it does not change existing behavior, it only adds correct logic for returning deferreds which follows existing practices and promises implementations (I'm familiar with 5 of them in JS), even if it's non-standard.

Methods done/fail should be thought of as "short aliases to then", that's how people see them and that's the most useful.

I planned to submit a pull request, but @dmethvin suggested to post the ticket for discussion first before. Please tell what you think.

Change History (6)

comment:1 Changed 5 years ago by timmywil

Component: unfileddeferred
Keywords: 2.1-discuss added
Priority: undecidedhigh
Status: newopen

comment:2 Changed 5 years ago by jaubourg

done and fail are here to add callbacks without the overhead of then. If you make them chain like then there's simply no way to attach handlers without creating a new Deferred internally which costs time and memory.

Now, from an API point of view, a chaining done doesn't make sense since you can just use then with a single argument. As for a chaining fail, it would need to be a new method and I'm not sure of the actual value of that.

comment:3 Changed 5 years ago by iliakan

Hi jaubourg,

First, performance is rarely an issue when adding a callback to deferred. Also, current jQuery deferreds implementation is not performance oriented. I guess so, because there are faster ways to implement deferreds. There are faster deferred libraries. You could make the new .done/.fail/.then work faster, but I guess you just concentrated on 'good enough' performance and the nice jQuery-style API.

Second, about the API. Julian, I wrote a letter to you about 3 years ago when deferreds were introduced to jQuery explaining the same issue about .done/.fail/.then. You declined it then, but through years jQuery gradually follows this vector. You introduced .pipe as another method which does .then right. Now .then switched to .pipe as it should have been from the beginning. Please continue going the right way and finally fix .done/.fail.

I know you've done most of the current implementation. Please try to look at this API from a standpoint of an outsider. A person who looks at your API from the outside.

Methods .done/.fail really look like aliases to .then.

P.S. Imho the most comfortable implementation is when the value returned by the callback from .done(callback) is passed to the next .done(callback) unless it's an error, then it goes to .fail. And if it's a deferred, then wait for it. In this case there is a clear difference in chaining order between assigning callbacks one-after-another wiith .done(c1).fail(c2) or using .then(c1, c2) to set them together.

comment:4 in reply to:  3 Changed 5 years ago by jaubourg

First, performance is rarely an issue when adding a callback to deferred. Also, current jQuery deferreds implementation is not performance oriented. I guess so, because there are faster ways to implement deferreds. There are faster deferred libraries. You could make the new .done/.fail/.then work faster, but I guess you just concentrated on 'good enough' performance and the nice jQuery-style API.

Performance is rarely an issue unless you have to create a new Deferred each time you add a *single* callback. Hence why done, fail and progress do not create a new Deferred internally.

jQuery Deferred have detachable methods. This makes the cost of creating a Deferred much heavier than prototype-based implementations (both in time AND memory). This is not something you can optimize away once you made all the methods attaching callbacks create a Deferred internally.

Second, about the API. Julian, I wrote a letter to you about 3 years ago when deferreds were introduced to jQuery explaining the same issue about .done/.fail/.then. You declined it then, but through years jQuery gradually follows this vector. You introduced .pipe as another method which does .then right. Now .then switched to .pipe as it should have been from the beginning.

This is not at all what the conversation was about. In particular, there was no mention of done and fail. The only proposal there was for then/pipe to handle exceptions like "In *all* implementation of deferreds" (quote).

I still stand by the arguments I gave you back then (and to which you promised an answer that never came). Pre-emptively catching exceptions and promoting them as rejections ignores the very meachanics of a promise and creates all sorts of problems which, to this day, the promise comunity hasn't found any decent solution to (though some like to claim otherwise).

Back to the conversation at hand, if you remember, we had just introduced pipe as the async chaining utility because some in the project didn't want to break backward compatibility with the simplest then we had at the time. We eventually decided to break backward compatibility by aliasing pipe as then. The deciding point was that there was still a means to attach callbacks without the cost of creating a Deferred, namely done, fail and progress.

Not to mention that done, fail and progress allow for multiple callbacks to be added at once which an async chaining implementation would prohibit.

Please continue going the right way and finally fix .done/.fail.

There is no notion of right or wrong here. Different elements in an API will have different use. done, fail and progress are just as "right" as then. The original Promise/A proposal had an optional method to attach callbacks without async-chaining.

To make it clear, what happened previously was that the non async-chaining then was pointless because we alreay had done, fail and progress. If we make them async-chaining now, then we need the old then back in some form which is a step backward.

Also, let me re-iterate that an async-chaining done would be pretty useless since you can just call then with a single argument already.

I know you've done most of the current implementation. Please try to look at this API from a standpoint of an outsider. A person who looks at your API from the outside. Methods .done/.fail really look like aliases to .then.

I don't think outsiders are that confused. In 3 years, I don't remember having been asked about the behaviour of done, fail and progress in relation to async chaining. I suppose most developers are quite happy to have a simple and efficient means to attach callbacks to the promise. In comparison, I've been literally harassed on twitter, on irc, by emails and at conferences about not being Promise/A(+) compliant since day one :P

Interestingly, I think people understand done/fail all right but are more confused about then itself.

comment:5 Changed 5 years ago by iliakan

Hi Julian,

Indeed, prototype-based implementations are the fastest. I didn't look deeply into how to optimize current API yet, but surely if one wants to concentrate on performance then using prototypes is the best way to go.

Right now deferreds in jQuery are clearly not performance oriented. That's because people usually do not make long callback chains on frontend. At least not using .done. With .pipe(.then) - yes, sometimes, but deferred performance is not the bottleneck in this case.

About the API. In current API the result of callback is discarded. I would suggest passing this result to the next callback. That may break some code which relied on the old behavior of discarding the value, but in the next version which is called 2.1, that is probably doable. Or, of course, you may want to introduce another method for that.

When done/fail callbacks do not return anything, things will keep working as now, so they are still very simple. But now with the ability to pass results down the chain, much more powerful.

comment:6 Changed 5 years ago by dmethvin

Resolution: notabug
Status: openclosed

Per recent team discussions, we're moving towards Promise and I don't think we want to introduce other changes here. The current behavior is well established.

Note: See TracTickets for help on using tickets.