Add the "Thenable Assimilation Procedure". Closes #75.

[domenic]

This more fully specifies the manner in which thenables must be assimilated, which is useful both for clarity and for usage in future specifications, e.g. specifying the behavior of resolve in a promise-creation spec.

The recursive nature of the thenable assimilation is a change from the current specification; see #75 for more details.

[novemberborn]

Technically this conflicts with "If x is a thenable, run Assimilate(promise, x)", since that'll lead to resolvePromise being called multiple times. Perhaps it should be clarified that a) resolvePromise is whatever means of resolving the promise who's trying to assimilate thenable's state, and b) this restriction only applies to the current assimilation.

Note that without b), a broken thenable could do the following:

var thenable = {
  then: function(resolve){
    resolve(otherThenable);
    resolve(true);
  }
};

[domenic]

Hmm, this doesn't seem unclear to me. resolvePromise and rejectPromise are "local" to the current Assimilate(promise, thenable) call, and I think this is communicated somewhat clearly already...

[novemberborn]

Upon re-reading, thenable.then(resolvePromise, rejectPromise) does indeed imply that resolvePromise is "whatever means of resolving the promise", but it could also be taken to mean a resolve method from the promise's resolver.

The distinction is important, because it means you can't simply pass resolve to the thenable as that may lead to the scenario outlined above.

[domenic]

Right, we don't define resolvers in this spec; the entire behavior of the resolvePromise function is defined a few lines above.

[briancavalier]

It was fairly clear to me, although it may not hurt to provide a very short description of what resolvePromise and rejectPromise are/do. May not be necessary, tho.

[novemberborn]

Implementation from line 68 in https://github.com/novemberborn/legendary/blob/master/lib/Promise.js#L68.

[briancavalier]

I was about to suggest here that we should be explicit here about calling rejectPromise(exception), but actually, I like it the way it is. This current language allows the implementation to use any implementation-specific means to reject promise, which is nice.

[domenic]

s/Assimilation/Adoption??

[briancavalier]

"Transmogrification" ftw ... j/k I use "assimilation" myself, so I prefer it (mostly out of habit?). It has a stronger implication of "become like" in my mind, but "adoption" can be similar, e.g. "adopting local customs".

[domenic]

Slightly updated. Would like to merge by this weekend and push out the 1.1 release; let me know what you think.

[domenic]

Would especially appreciate @lsmith's wordsmithing :)

[juandopazo]

I don't quite follow the difference between how a promise and a thenable are assimilated. Are we assuming promises will have a synchronous way of getting its result or other ways of listening to its state change other than then?

[juandopazo]

Another point. Although I don't think we need to standardize "resolvers", the spec would be a lot easier to read and parse if we added "Resolve", "Fulfill" and "Reject" as algorithms/functions. That way instead of writing 600 times stuff like "If resolvePromise or rejectPromise have been called, ignore it." we can just say "call Reject(thenable)" or something like that.

I'll try to write something in the style of the EcmaScript spec.

[lsmith]

I'll take a look at this tonight. Sorry I haven't been able to get to it until now.

[domenic]

I don't quite follow the difference between how a promise and a thenable are assimilated. Are we assuming promises will have a synchronous way of getting its result or other ways of listening to its state change other than then?

Yes, exactly that. An implementation-specific way, if nothing else. Did you see the footnote?

[domenic]

Although I don't think we need to standardize "resolvers", the spec would be a lot easier to read and parse if we added "Resolve", "Fulfill" and "Reject" as algorithms/functions.

I see where you're coming from, and somewhat agree. But, my gut feeling is that this would hurt the simplicity of the current spec, which is why I didn't try it. Even the Thenable Assimilation Procedure section seems a bit out of place in the current spec, which has this nice explanation of the then method as a series of bullet points, but then out of nowhere delegates to this subprocedure with pseudo-code notation like Assimilate(promise, x).

Happy to be proved wrong though!

[domenic]

Are the following inconsistent?

If/when thenable is fulfilled, promise must be fulfilled with the same value.

vs.

If/when resolvePromise is called with a value x,

1. If x is not a thenable, promise must be fulfilled with x.
2. If x is a thenable, run Assimilate(promise, x).

It seems like if an implementation allows promises-for-thenables, they are. Implementations should not allow such things, but that's outside the scope of the spec.

Maybe it should be revised to

If/when thenable is fulfilled with a value x,

1. If x is not a thenable, promise must be fulfilled with x.
2. If x is a thenable, run Assimilate(promise, x).

[rkatic]

@domenic Wouldn't "resolved" instead of "fulfilled" make much more sense? Or they become synonyms in 1.1? I'm a little confused here.

[domenic]

@rkatic indeed, they are synonyms when x is not a thenable. We have avoided the term "resolved" or "to resolve" in the base Promises/A+ spec, which I think is good for simplicity. The promise creation spec, which is somewhat settling on promises-aplus/constructor-spec#18, introduces the concept of resolving.

[rkatic]

I see. However I would be more in favor for consistency, or at least not having contradictions between specs. Am I wrong on seeing any?

[domenic]

What is the contradiction you are seeing? Maybe quoting a specific line would be more helpful.

[rkatic]

I am referring to:

If/when thenable is fulfilled with a value x,

I would expect "resolved" here, even if there are no clear distinctions between those two words in this single spec.

[domenic]

What other line of this spec does that line contradict?

[rkatic]

I am not referring to contradictions inside this spec alone, but eventually between this spec and the resolver-spec.

[domenic]

In that case, what line of the resolver spec does that line contradict?

[rkatic]

If I am right, according to the terminology in promises-aplus/constructor-spec#18 (first section), "fulfilled" is an already "Settled" promise, therefore the "Resolved" term should be used here.

[domenic]

We don't want to fulfill promise when thenable is resolved; we want to fulfill it when thenable is fulfilled. If thenable is resolved to a non-thenable, these are equivalent; if thenable is resolved to another thenable, we first wait for thenable to be fulfilled, then fulfill promise with that value.

[rkatic]

I am aware of that (or missing something) and still consider "fulfilled" kinda wrong here. Thanks for trying anyway.

[ForbesLindesay]

This seems to limit the extent to which you can assimilate a promise. Specifically I think this might be worded in such a way as to prohibit progress propagation and cancellation propagation. It might just be my reading of it though. Perhaps the wording needs to be clearer?

[domenic]

@ForbesLindesay could you outline the problem in more detail?

In general, I think it is problematic, since Q at least has the ability to mark promises as "don't try to resolve me" via Q.fulfill. The use case envisioned being objects that happen to have then methods, but should not be treated as promises. The wording here seems to prohibit that.

Also, I think this is important and hope we can figure out some good wording and push it out soon, since I really want to get it in to 1.1. I don't think it's feasible to release 1.1 with only the fixes we have, only to introduce a change like this shortly afterward. I'd like 1.1 to be the only real revision we end up doing. But it's blocked on this :-/.

[domenic]

We could add a footnote to the definition of thenable, that allows exceptions for objects implementations have marked as "not a promise" via some specific mechanism. Seems OK.

That just solves the problem I was talking about, not @ForbesLindesay's, though.

[ForbesLindesay]

When I call:

promise.then(function () {
  return foreignPromiseThatEmitsProgress;
}).then(null, null, function (prog) {
  console.log(prog);
});

I should see progress being logged, ideally.

If assimilate is just a simple implementation of the algorithm described, that won't happen. The implication of the spec is that doing anything else in addition to what's described isn't allowed, and so I couldn't extend the algorithm to also propagate progress. The same argument follows for cancellation.

[ForbesLindesay]

P.S. I'd quite like this spec to reserve the third argument for progress use in this spec at some point.

[rkatic]

The marking of thenables as "not a promise" is unpractical since different implementations would use different "markers".

[ForbesLindesay]

I really hate the idea of supporting "thenables that aren't close enough to promises to be assimilated" - I've never seen one. If you do see one, handle it by explicitly converting it to something that's not "thenable" before it can be assimilated.

[ForbesLindesay]

To help make clear how much of a non-issue I think thenables that need to be passed through should be for this core spec I've created an extremely robust wrap/unwrap solution called thenable which can be seen here. It's 16 lines, and that's only because it goes out of its way not to unwrap things that it didn't wrap up.

[rkatic]

@ForbesLindesay At certain extent, I understand your "hate", but not allowing thenable fulfilled values would make Promise/A+ incompatible with other promise specs that allows that. Using a wrapper would certainly complicate usage of a Promise/A+ implementation with others.
Also, this spec seems to allow thenable rejected reasons, and not allowing thenable fulfilled values seems kinda odd.

[rkatic]

Would it really be so bad to also have a fulfillPromise that is perfectly symmetrical with rejectPromise? It seems so obvious to use it along with rejectPromise, and to use resolvePromise when resolution is required.

[ForbesLindesay]

You have 3 options:

1. Mark thenables
2. Mark promises
3. Wrap thenables

Marking thenables is fine with me, except that it would be difficult to ensure interoperability (we'd have to agree a marking strategy)

Marking promises is a non-starter, there are too many promise implementations and part of the point is to do our best to assimilate promises that weren't necessarily designed to be perfect promises/A+ promises, they're just moderately well behaved thenables.

Wrapping promises is fine, it has no overhead for anyone except those people who have decided to use thenables that aren't promises along with a promise library (I suspect that may be the empty set unless you're in it). The compatibility issue with other specs you suggest is a non-issue since we're already incompatible with http://wiki.commonjs.org/wiki/Promises/A which ought to be the easiest one to be compatible with. What we need is compatibility with what people commonly do, and I think this provides that.

As for fulfillPromise the idea has been floating around for a while in the resolvers-spec with little support. It still wouldn't really help you though as you'd be able to create a promise for a thenable, but that doesn't mean you could return it from a .then function. The point at which fulfill was given most serious consideration was when it was suggested that it could throw a TypeError when passed a thenable, which would completely prohibit your use case anyway.

In short, creating promises for thenables is incompatible with the goal of making it impossible (or at least very difficult) to create promises for promises.

As a final point, you could call thenable.unwrap for every object that gets returned from then callbacks after checking to see if it's a thenable. You'd be incompatible with the spec, but you would at least pass all the unit tests and have the behavior you desire. Unless someone inserts a test that returns a Wrapped and checks it doesn't get unwrapped of course.

[rkatic]

..., but that doesn't mean you could return it from a .then function.

Not sure what you mean here. Could you show a simple example?

If you start with the goal that there should not be promises for promises, then of course that a fulfill (that accepts anything) will not be supported. I am wondering why promises for promises is so wrong. Maybe these are not practical to you, but I am sure it can be, not just for me. Promises that fulfills with itself seems useful in many situations where promises/thenables own different informations and results (thenable xhr, for example).

[ForbesLindesay]

The case of thenable being a promise never truely happens unless your consuming one of your own promises, so the procedure for all other promises is as follows:

1. Otherwise, call thenable.then(resolvePromise, rejectPromise), where:
   1. If/when resolvePromise is called with a value x,
      1. If x is not a thenable, promise must be fulfilled with x.
      2. If x is a thenable, run Assimilate(promise, x).

function fulfill(val) {
  return {then: function (onFulfilled) { onFulfilled(val); } };
}

promise
.then(function () {
  return fulfill(fulfill(fulfill('foo')))
}).then(function (val) {
  assert(val === 'foo');
});

assimilate just gets called 3 times until it's finished unwrapping the promise, then it's done.

The behavior you want is:

1. Otherwise, call thenable.then(resolvePromise, rejectPromise), where:
   1. If/when resolvePromise is called with a value x,
      1. promise must be fulfilled with x.

That would let you have:

function fulfill(val) {
  return {then: function (onFulfilled) { onFulfilled(val); } };
}

var sentinel = fulfill(fulfill('foo'));

promise
.then(function () {
  return fulfill(sentinel )
}).then(function (val) {
  assert(val === sentinel);
});

The fulfill method is trivial, nobody's stopping you creating a promise for a promise, and nowhere in the spec prohibits it. What the spec does prohibit is getting one of your promises for a promise assimilated into someone else's library.

[rkatic]

I think you have illustrated correctly what I was talking about, beside that resolvePromise in the example would be named fulfillPromise...

What the spec does prohibit is getting one of your promises for a promise assimilated into someone else's library.

Why is this important?

[ForbesLindesay]

Because promises for promises are generally considered bad. This is a fact that has been discussed at length in many other issues. I'm fine with bad things happening in your library, as long as I have a method to ensure that they don't cross the boundary into my library. If the spec prohibits me from preventing your bad stuff from entering my library then I have a problem.

As for why promises for promises are bad: They make debugging harder by adding an un-necessary layer of nesting. They have a tendency to "escape" into areas of an API that weren't supposed to have promises in them. For example promised functions that are intended to automatically resolve their own arguments or code that uses node.js style callbacks.

It also breaks the say what you mean rule:

A promise represents a value that may not be available yet.

A promise for a promise represents that second promise, which is already available, because promises always are (you can create a new promise that can act as a proxy, so is equivalent). As such, you're not saying what you mean.

A wrapped promise represents a promise that should not be waited on yet and must first be explicitly unwrapped

That by contrast does say what you mean.

[rkatic]

They have a tendency to "escape" into areas of an API that weren't supposed to have promises in them.

This is not exactly a problem, in my opinion, because my fulfilled value is not intended to be used as a promise any more.

The real problem is when such areas, at some point, do support promises, and the value is used as a promise again. Even if this is not a problem with promises that fulfills with itself (if assimilation would work in my way), it still remains a problem in other cases.

A perhaps even bigger problem, is thinking to "sanitize" a thenable by recursively resolving it. Not only it would stuck on resolving a cyclic thenable, but also it would try to use a fulfilled value as a promise, even if it is not intended to be used as a promise any more.

I'm not sure that wrapping promises is a valid solution. Once the promise is unwrapped for usage, it could "escape" again. Keeping something "constantly" wrapped could be a real challenge or a too big pain to do.

At this moment, I think the less dangerous solution is to make user responsible for not making such thenable values "escape", instead of giving a wrong sense of security.

[rkatic]

I am aware that I am new here, and that because of many already conducted discussions, a huge (?) majority here have already took an (almost) clear position on how things should work in this spec.
Because of that, most of my questions and oppositions, are probably annoying you. I am grateful for your patience and effort given.
Now my only concern is that currently the majority (?) of Promise/A+ implementations is only partially adopting recursive assimilation, and that upgrading to 1.1 would probably bring some back-compatibility issues.
I think, that the only valid strategy to keep compatibility would be wrapping in not promises (marking should be avoided entirely for interoperability reasons). Simply renaming Q.fulfill would certainly not be an valid solution (kriskowal/q#224). Correct me if I am wrong here.

[ForbesLindesay]

I think you're correct. As far as I'm aware it's an API that's rarely (if ever) used to actually create promises for "thenables which aren't promises though" so it would just be a case of making the change and jumping in to help firefight issues people have as they arise (there would be a few scattered here and there).

If we can get buy in from key players (especially @kriskowal as Q is the only library that I know does this) then I don't see any problem with moving ahead down that path, the focus now needs to be getting consensus among implementers.

[rkatic]

As far as I'm aware it's an API that's rarely (if ever) used...

Unfortunately, it seems that it is used (or considered useful), and that it's the reason of not removing Q.fulfill yet (kriskowal/q#205 (comment)).
I think it is reasonable to expect from Q to not introduce back-incompatibilities until next major version (1.0.0).

[briancavalier]

I haven't been able to keep up with this thread as closely lately, sorry guys. @rkatic and @ForbesLindesay: It's not clear to me what's being proposed in these few messages relating to Q.fulfill and fulfilling promises with thenables. Could one of you summarize what you're proposing? Thanks!

[rkatic]

@briancavalier You didn't miss anything important I guess.
With this change, and recursive assimilation, it would be impossible (or very hard) to have promises for thenables.
This could make the transition from 1.0 to 1.1 much harder because of various back-compatibilities issues.
I was for non recursive assimilation, replacing resolvePromise with fulfillPromise (I am new here, sorry), but thanks to @ForbesLindesay, and his patience, I am now ok with this change.

[briancavalier]

No worries, @rkatic. Thanks for summarizing.

[domenic]

I realize that this thread has gotten a bit sidetracked in places, but I've seen no outstanding issues, and think it's time to move forward and publish a 1.1 with this change.

With that said, I'll merge this change, and start a new issue asking for sign-offs from implementers and other involved parties before we officially publish a 1.1, i.e. update the gh-pages branch.

[rkatic]

I gave another look to the change, and it seems I forgot something during discussions above.

[1] This change allows Promises fulfilled with anything (values propagated as they are).
[2] This change doesn't allow thenables fulfilled with thenables (recursive assimilation).

Conclusion: Every Implementation can have promises for (fake) promises [1], but it is not allowed to tollerate those from other implementations [2]. -> Coercing broken by spec.

Please tell me that I am missing something.

[domenic]

@rkatic that is by design. Good implementations will not allow promises-for-thenables, but most importantly, as @ForbesLindesay has explained to you already, at the boundary we cannot allow thenables-for-thenables from other libraries to infiltrate well-behaved libraries. That is, implementations are not allowed to tolerate thenables-for-thenables; chains must be squashed at the boundary.

[rkatic]

Ok, but isn't the "squashed at the boundary" strategy potentially dangerous if the fulfilled value is thenable but does not behave as a promise? Is this chosen because Assimilations should never throw? In that case, I would probably prefer rejecting the promise...maybe...

[domenic]

If you are using promises with thenables that don't behave as promises, the answer is "don't do that." Unfortunately there's not a better answer (see a good summary here).

In particular saying "you should hide your bad-thenables as the fulfillment value of fulfilled promises" is a much worse answer than "you should hide your bad thenables inside a non-promise-related wrapper object."

[rkatic]

Is the "don't do that." thing specified somewhere in the spec? If not, maybe it should...