-
Notifications
You must be signed in to change notification settings - Fork 165
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
Done #43
Comments
I'd prefer not to include |
Agreed. This is outside the scope of
Maybe we can include |
I think something like Each promise implementation should have a function that escapes a chain and frees unhandled errors, I explained my reasoning also here: http://mozilla.6506.n7.nabble.com/Promises-td101158i20.html#a101162 |
@medikoo Believe me, I completely agree with you. That's why I implemented exactly that in Q :). But as I said, outside the scope of Promises/A+. |
@domenic I think it should be part of Promises/A but it's ok, I take your answer :) |
My reasoning for wanting it in the spec, as opposed to as a separate thing, is that we'd use it in libraries all the time if we could depend on it: function all(promises) {
var def = defer();
var arr = new Array(promises.length);
var waiting = promises.length;
promises.forEach(function (promise, i) {
promise.done(function (val) {
arr[i] = val;
if (--waiting == 0) def.resolve(arr);
}, def.reject);
});
return def.promise;
} That bit of code is typically written using I'll accept it as not part of this spec, but I think it should be part of the near future. I personally think we should aim to specify as many of the methods that often appear on promises implementations as possible, one at a time in separate spec's. |
In the light of Firefox implementing done(), should we reconsider adding it to A+ to pave the way for DOM/JS specs? It would also help as get the right behavior we want from the standards groups. For example, Firefox's done() doesn't throw when a rejection callback is omitted. |
Firefox has deviated from the DOM promises spec in several ways; I think they're just lagging. |
I think you're right. But theoretically, if we were to agree on something, would we agree on this (expressed as if part of the DOM spec)? The done(fulfillCallback, rejectCallback) method must run these steps:
|
Ugh, I hate reading DOM spec writing... I don't think that's correct. It doesn't handle the case where Previous discussions at promises-aplus/unhandled-rejections-spec#5. |
In the context of the DOM spec, it does because it doesn't create a "promise wrapper callback" for each callback, which would catch exceptions. It just add callbacks to each callback list as they are. |
You guys know how I feel about |
I'm largely in favour of I would specify it as
|
@ForbesLindesay that creates an unnecessary intermediate promise and it forces an extra tick which may not be necessary. But I guess we agree that it must throw if no reject callback is passed and it returns undefined. I'm looking at the unhandled rejections monitor in |
The approach we're taking in RSVP is to install an unhandled promise monitor that throws by default. You can opt a particular promise out of this behavior by attaching a noop failure handler, if you know that you will be attaching asynchronous error handlers. We will probably have sugar for this ( In our experience, moving the burden from literally everyone to people who may want to attach async error handlers is appropriate. |
@domenic @briancavalier @wycats There are few very common use cases, that I think must be addressed by promise spec:
Those uses cases are in a dead simple and straightforward way solved by Instead you propose that official specified solution for that should be using
So cutting it short: instead of giving simple and possible solution A, You suggest using solution B which raises 2 important issues, and propose a complicated and not complete solution C which will just address one of them. How logical is that? :) I totally don't buy the argument that deciding whether to use Both are different and serve different and clear use cases. Same with array iterators, we have |
@medikoo The way In Ember, a common use case for promises is the return value of the model hook: App.IndexRoute = Ember.Route.extend({
// if the return value of this method is a Promise, Ember will wait for it to resolve before
// moving further down the route hierarchy and before rendering any templates
model: function() {
return $.getJSON("/me").then(function(profile) {
return $.getJSON(profile.photosURL);
});
}
}); As you can see, it is inappropriate to tack In this case, it would obviously be disastrous (and broken) to change the final The advantage of Promises is that they create a single, straight-forward API that totally replaces the callback-based API. In my experience, people's initial brush with promises is not very sophisticated, and adding in In libraries that are mostly used by people who have opted into promises and learned about them, I can easily see how adding more learning by way of |
@wycats sorry that's a really bad example. The way |
@domenic Sorry, but that isn't how people explain |
Source? |
My proposal (and it sounds like, @juandopazo) is to keep the promise usage API simple (always This shifts the burden from everyone to a much smaller group of people who have a somewhat exotic use-case. |
@domenic http://msdn.microsoft.com/en-us/library/windows/apps/hh700334.aspx The first set of rules doesn't mention your golden rule. It says:
You can go get Microsoft to fix this particular phrasing, but that isn't my point. It's really hard to propagate a rule throughout the ecosystem that is both accurate (and doesn't hit the issue I raised) and doesn't fall victim to a game of telephone. You understand promises very well, so the rule seems obvious. Remember that for most people, the idea of a first-class eventual value is very new, and they're just following simple instructions to get their job done. |
We've been over this before, but to just reiterate it for the record: The use case of attaching handlers more than one tick after the fact is not at all exotic, and is in fact crucial to the composability of promise systems and the ability to refactor them without fear. You will one day upgrade your third-party promise-accepting library and find it completely broken for "no reason," only after much debugging realizing that you now need to
|
My bottom line: |
To be clear, I still think unhandled rejections is a huge problem, and |
@domenic I am strongly in favor of tools. The The primitive .onerror in RSVP is a starting point towards better tools as well. |
@domenic perhaps we're closer together than we realized? |
Then I think we are in complete agreement, and we'll work as fast as we can to get those tools ready for you ^_^ |
@juandopazo do you mean something like onerror? Or visual tooling? It's not really up to those specs to specify how visual debuggers should work. |
Exactly
This is very different use case, it's not end of promise chain, you actually return promise for further processing. I advise you to look out of Ember into more direct promise usage e.g. how it's done in plain Node.js scripting, over there use case for |
@medikoo I've, of course, used promises in many other use-cases. I understand why the (overly simple) advice is common and works for many people. My point is that if you're the author of an abstraction that requests that the user HAND you a promise, this popular advice will cause you no end of pain, because it's overly simple and doesn't capture the nuance. Once you explain it properly, it's very hard to make sure that everyone who describes Even if you did explain it properly, it now means that users have to think about this choice in every usage of promises, making a simple API ( I fully, 100%, understand the pain that swallowing errors causes. This has been the source of a lot of frustration by Ember users, and I agree that a solution is required. However, I believe that printing unhandled exceptions to the console in the very short term, followed by better tooling in the medium term, will solve the problem without making the API more unpleasant to use. |
Just putting this out there:
People struggle because we don't teach it as:
We're teaching the two methods the wrong way round. rant start Code is written once, maybe twice and then read tens or hundreds of times and executed thousands of times. The importance of really clear signaling of intent and early and LOUD failure cannot be overstated. No unhandled rejection debugging tool can shout as loudly as actually calling rant over, sorry |
@ForbesLindesay - C# also has the Reactive Extensions library (Rx) produced by Microsoft, which (to the best of my knowledge) is built on top of Task and is meant for the same kinds of things as Promises. The primitive is I dislike having to use |
Reactive Extensions are not built on top of tasks, they are much more similar to node.js's Stream class. The task related methods are mostly just let you do things like wait for the next value in the IObservable (Reactive's equivalent to a readable stream). Yes, we've all had occasions where we forgot to add a
|
@ForbesLindesay my hunch about why it's not confusing is that "don't swallow errors" is the default in C#. If you want more advanced behavior, you learn about and opt into error swallowing. With JS promises, we chose (for good or ill) to error swallowing the default. That forces us to find better solutions that help users avoid those hazards. Perhaps a better initial solution would have been .then with the .done behavior, and a .chain or .pipe for the current behavior. I believe jQuery experimented with that. Again, I think we're too far down the road to change the core semantics now. RSVP is choosing, for now, to throw unhandled exceptions via the default behavior of an onerror hook that gets invoked for unhandled exceptions. I hope we have a better solution before @domenic's doomsday scenario comes to pass. |
Of course it's too late to change the It's not too late to make The issue is that at the moment we're teaching people that |
.done is a kind of silly name for the main, default primitive, and making sure people get the right guidance here seems really hard. I'm not changing semantics; just providing a hook with convenient defaults for the current situation. |
@ForbesLindesay This is a very clear explanation, and is the first time I've thought My experience in seeing people use promises matches @wycats', though. I see people make mistakes with That said, neither
|
I really doubt, that it's confusing for developer to learn when to use Difference between |
@medikoo amen, you arely see people mistakenly using Think back, did anyone learn to use @briancavalier I agree with all of this. I think we would all do well to remember "they aren't mutually exclusive". I still think the problem ConclusionI think we should have a separate repository for a "Promise Chain Termination" spec. By separating out the specifications for |
P.S. @wycats you are leaving it as possible to opt-in to Promises/A+ semantics by replacing the default error handlers, but your default behavior is not Promises/A+ compatible. This is why you are changing the semantics. |
I'm against another repository for this. I think we already agree on the semantics of @wycats et al, my point previous point about DOM promises and debugging was that I don't see DOM promises having any sort of monitor for unhandled promises, so |
Interestingly, I find the opposite to be surprisingly common: I see people using Perhaps this means that if I'm not really sure where this leaves @ForbesLindesay I think the "devs won't enable tooling" problem could be solved in much the same way as teaching devs how to use
@juandopazo Do you know what plans they do have for helping developers to debug? It would def be interesting to know. |
I do think we will see developers jumping through hoops to get the function getJSON(url) {
return new Promise(function (resolve, reject) {
get(url).then(function (res) {
resolve(JSON.parse(res))
}).then(null, reject)
})
} which is just painful, I'd rather people started out with That seems to me like a natural learning curve. At each stage, the tools you get seem better than what you had before with few obvious downsides. I think a lot of people will get put off by If we all (roughly) agree on how |
While that may be a good rule of thumb for new promise users, we shouldn't think of chains of promises only. Since multiple handlers can be attached on a promise, I like to think of them as trees (basically we could build all kinds of acyclic graphs) and @juandopazo wrote:
I would like
|
@bergus I know that chaining is standard practice, but IMO in this case returning anything other than |
@bergus Calling What you're asking is something between |
@briancavalier that is also my experience. I teach an introductory Ember course for people with jQuery and JS experience, and I see a lot of confusion about |
@bergus Yep, me too. We should probably use "tree" when referring to a promise graph/subgraph rooted at a particular starting promise, and "chain" to mean a single path from said starting promise to a leaf (which does indeed have an end ... at the leaf!). I see the utility in making |
Do you mean "except @wycats"? The process of persuading a recalcitrant in order to gain consensus is useful. At the very least, it will force us to explore parts of the solution space that we've been ignoring. |
@wycats yes, my comment was more about the fact that if "we mostly agree" (as @juandopazo suggested) then "it would definitely be a good time for a spec". I actually equally think if the vast majority are settling on one thing and an implementation or two does something different, that's probably a good indication that we need to be discussing the differences and seeing what we can learn from each other. My current thoughts are that Also, I think @bergus example: function getJSON(url) {
return get(url).done(console.log.bind(console, "original response:")).then(JSON.parse);
} would be better as: function getJSON(url) {
return get(url).asside(console.log.bind(console, "original response:")).then(JSON.parse);
} which would desugar to: function getJSON(url) {
return get(url).then(function (res) {
console.log("original response:", res);
return res;
}).then(JSON.parse);
} I see no reason you'd want the error throwing behavior of |
Yup, that's precisely why As for why I'm against another repository, it's because of how things have evolved since A+ started. Now there is almost certainty that promises will be in EcmaScript and we already have prototype implementations in the DOM. I know some people will prefer to keep their own implementations even when native ones are available, but I honestly see no reason why all of our libraries shouldn't become wrappers and utilities for native promises. This means that the role of A+ should probably change. It's no longer about reaching consensus between web developers. Now there are two other standard bodies writing a spec for something that will be automatically available for all developers. So I don't think there's a point in having texts for optional features like progress when the native versions won't provide them. Given that Firefox has removed |
The way I see it, the role of the Promises/A+ organisation is two fold:
Point 1 clearly becomes less important as we get native promises in the language / in the DOM. Point 2 does not. Point 2 is partly important for users of promise libraries who want to know exactly how their library will behave. It's partly important for external standards organisations though. The Promises/A+ method of producing a specification has proved extremely effective for producing a very useable API and a very easy to read spec (relative to other specifications). As such I think it's a great idea for Promises/A+ to continue to specify the features that most promise implementations agree on. By doing this, we can help encourage TC39 and W3C etc. to produce nice specifications that include the features people want. |
That's my reasoning too, but I'm taking an extra step: if our goal is to encourage TC39 and the WHATWG to include certain features, we should all agree on those features being included. Having optional features doesn't help get our voice heard. |
ok, that makes sense, I actually kind of like the "one repo" approach. I think it would be an interesting experiment to try and get the |
Could we define the behaviour of the
.done
function? I really think all promises implementations should include this method, and it would be great if they were consistent.I don't know if it belongs as part of this spec, or a separate spec, but I want to be able to say "The object has a
.then
method so it is a promise. The promise has a.done
method, so.done
will ensure errors are thrown."The text was updated successfully, but these errors were encountered: