promises-aplus / constructor-spec

Discussion and drafts of a possible spec for creating and resolving promises
10 stars 4 forks source link

Fulfill, reject, adopt? #12

Closed domenic closed 11 years ago

domenic commented 11 years ago

In #11, I outline how the polymorphic resolver API is pedagogically problematic. I still see the attraction of disallowing promises-for-promises though. Could we perhaps do so as follows?

We'd probably also need resolve(x) which doesn't throw, since that behavior is useful. On the other hand, I don't think a polymorphic reject is useful behavior, even if it is proposed in promises-aplus/promises-spec#66.

ForbesLindesay commented 11 years ago

If I was going to go that route I'd be inclined to drop adopt altogether. Having something that's polymorphic is useful, but you could replace adopt p with just p.then(fulfill, reject) so I don't see that it adds much value. I think either we should specify polymorphic resolve and break and not specify fulfill/reject or we should specify fulfill/reject and not let you pass a promise to them.

briancavalier commented 11 years ago

I'm obviously a big fan of the polymorphic resolve/reject right now, but I'm also a fan of simple, so I definitely think this is worth talking through.

Aside from the polymorphic resolve/reject preventing promises-for-promises, one thing I like about them is that, at least in my mind, they kind of just "do what I mean". Pass anything to them, and they work it out, and personally, I prefer resolve(otherPromise) over otherPromise.then(fulfill, reject). I fully admit that it may not seem like "doing what I mean" to folks who are accustomed to using a non-polymorphic implementation.

As @ForbesLindesay pointed out, since adopt(p) is equivalent to p.then(fulfill, reject), it is possible to implement polymorphic resolve and reject using only isPromise(x), fulfill(x), and reject(x). Given that, would it make sense to specify fullfill/reject in the way @domenic described above, and libs could provide polymorphic methods if they want?

One thing that bothers me about that is polymorphic resolve matches well with then()'s now deeply-entrenched polymorphic return. Making return non-polymorphic would (obviously) be a disaster. We'd also still need to retain polymorphic throw ... which, of course, matches well with polymorphic reject :/

Another oddity is that we'd be allowing fulfill/reject to sync throw to indicate you violated a precondition, but we decided not to do that for then(). Probably not a huge deal, but worth mentioning.

briancavalier commented 11 years ago

Some things that seem important:

  1. Treating resolver verbs in the same way as return and throw. For example, if we have fulfill(verbatim) but return is polymorphic, it creates imbalance between handlers at the beginning of a promise chain and those that come later ... and thus:
  2. Ensuring handlers behave in the same way regardless of where in a promise chain they fall. Otherwise, we create refactoring hazards and other odd situations documented in promises-aplus/promises-spec#65.

So, my thinking is that ideally we should either disallow promises-for-promises in all cases, or allow them in all cases.

Preventing promises-for-promises

  1. Polymorphic resolve, reject, return, and throw. There are pedagogy questions here, possibly even moreso with polymorphic reject as @domenic mentioned in #11.
  2. Polymorphic return and throw, but non-polymorphic fulfill and reject that must not accept a promise--they must sync throw when passed a promise. This creates some asymmetry between fulfill and return, and between reject and throw. Developers are living with this asymmetry now in many cases--I don't know how much it hurts the learning process.

Allowing promises-for-promises

  1. Non-polymorphic fulfill, reject, return, and throw. The big problem here is that polymorphic return is one of the most entrenched promise behaviors. This also raises more questions, for example, what does return rejectedPromise do? Presumably the next promise in the chain would be fulfilled with rejectedPromise as its fulfillment value.

    I haven't thought through the implications of this one, but ditching polymorphic return seems like such a big problem that I don't know if we could ever get this to fly. Seems like it'd require a lot of rethinking of the entire mechanics of thenable promises.

domenic commented 11 years ago

Pass anything to them, and they work it out

How often do you take advantage of this? That is, how often do you have a value that you're not sure whether it's a promise or a non-promise? Personally, I can't think of a time that's ever been true for me.

would it make sense to specify fullfill/reject in the way @domenic described above, and libs could provide polymorphic methods if they want?

It's becoming increasingly clear to me that the resolvers spec is going to have a "normative optional" section. That is, implement this core, and if you want to implement this other sugar, implement it like we say. For example if we went with a minimal proposal like in #10, we'd probably want to spec Promise.reject or a reject parameter.

Another oddity is that we'd be allowing fulfill/reject to sync throw to indicate you violated a precondition, but we decided not to do that for then()

At first I had this doubt too, but then I realized that the reason I was so strongly against throwing with then is that then's behavior should, in my opinion, be entirely asynchronous. On the other hand, fulfill/reject are---I would think---synchronous operations, i.e.

var p = new Promise(function (resolver) {
  resolver.fulfill(5);
});

// Promise gets fulfilled immediately and we can synchronously inspect that:
assert("value" in p && p.value === 5);

p.then(function () {
  // But this callback is stilled called in a future turn.
});

This creates some asymmetry between fulfill and return, and between reject and throw`.

I don't think these things were ever symmetric to begin with, especially after the introduction of polymorphic throw as per recent discussions. return could result in a rejected promise, so saying there's a parallel to fulfill seems wrong.

And yes, I think ditching polymorphic return is impossible.


The more I think about this idea, the more I like it. It avoids so much complexity but still prevents promises-for-promises.

ForbesLindesay commented 11 years ago

I agree with @domenic on everything except:

Pass anything to them, and they work it out

I have occasionally used that, but I could definitely live without it.

This creates some asymmetry between fulfill and return, and between reject and throw

If you replace fulfill with resolve and consider that some libraries might already have a polymorphic reject and this makes more sense. If we spec'd polymorphic resolve and polymorphic reject we would have really nice symmetry.

Even if ditching polymorphic return were possible I think it's one of the nicest features of promises in JavaScript vs. other languages. Having to add a call to unwrap promises each time is such a pain.

domenic commented 11 years ago

If we spec'd polymorphic resolve and polymorphic reject we would have really nice symmetry.

Well, as I said in https://github.com/promises-aplus/resolvers-spec/issues/10#issuecomment-12579958, if it's polymorphic it can't be called reject.

ForbesLindesay commented 11 years ago

It seems like polymorphic throw is no longer likely, in which case we shouldn't have polymorphic reject.

domenic commented 11 years ago

Indeed, I don't think this is valuable anymore as-written. (resolve, reject) seems to be the winning pair, modulo #9.

If we were inclined to specify an optional fulfill, though, it should be specced to throw on being given a promise, I think.