Make Resolve operate the same on promises and thenables, using `then`

[domenic]

Proposal

• Remove special-casing of IsPromise in Resolve. Instead, do one-level thenable unwrapping there. That is:
• Upon calling schedule a microtask do the usual get-then, call-then, dance.
• The details here are a bit complicated but we should take whatever we get called back with, and do no more unwrapping beyond that first level.
• Promise.prototype.then still does recursive unwrapping.

  Observable Changes

• This removes the internal state introspection of true promises done by the current Resolve algorithm, instead using the other true-promise's then:

  const p = new Promise(r => r(1));
  p.then = f => f(2);
  Promise.resolve(p).then(x => console.log(x)); // logs 2 now

  See related #53.

• This makes overriding then in a promise subclass do what you expect when you return the promise:

  class InstrumentedPromise extends Promise {
    then(...args) {
        console.log("called then!");
        return super(...args);
    }
  }
  
  const instrumented = new InstrumentedPromise(r => r("instrumented value"));
  
  Promise.resolve(5).then(() => p).then(x => console.log(x));
  
  // Before this change, would only log "instrumented value".
  // Now, logs "called then!" then "instrumented value".

• Implementations now make an observable call to then whenever a promise is resolved with another true promise, whereas previously they reached into its internal state.

  Other Concerns

• This seems to give a nice simplification and decoupling, making the internal data properties used only for tracking of internal state and not for cross-promise communication.
• This should not impact monadic promise plans as we only do one-level unwrapping; hypothetical monadic promises for promises for promises will be turned into monadic promises for promises, allowing flatMap to retrieve the inner promise.

[domenic]

@erights would appreciate your feedback before I implement this change; assigning to you.

[wycats]

Here are some quirks that these semantics improves, both in the old semantics:

promiseSubclassA.then(function() {
  return promiseSubclassB;
}).then(function(val) {
  // promiseSubclassB.then not called
});

spawn(function *() {
  var a = yield promiseSubclassA;
  var b = yield promiseSubclassB;

  // promiseSubclassB.then called
});

promise.then(function() {
  return promiseSubclass;
}).then(function() {
  // promiseSubclass.then not called
});

promiseSubclass.then(function() {
  return promise;
}).then(function() {
  // promiseSubclass.then called (obviously)
});

[erights]

assigning to you

accepted

We've discussed this before, I think over at Promises/A+. Without the special case, you lose the ability to support promise pipelining and lazy evaluation. Also, I don't see how this could possibly support .flatMap -- please explain.

[domenic]

Without the special case, you lose the ability to support promise pipelining and lazy evaluation.

I am not sure I understand how this impacts pipelining, but, I do see the problem for lazy evaluation. Hmm. On the other hand, I am not sure how lazy evaluation fits with the current spec either, as we eagerly reach into the promise's current state upon resolving.

Also, I don't see how this could possibly support .flatMap -- please explain.

Let us assume there is a Promise.accept and a Promise.prototype.flatMap. Then:

Promise.accept(5); // Does no unwrapping, promise for 5
Promise.accept(Promise.accept(5)); // Promise for promise for 5
Promise.resolve(Promise.accept(5)); // Does one-level unwrapping, promise for 5
Promise.resolve(Promise.accept(Promise.accept(5))); // Promise for promise for 5

Promise.accept(Promise.accept(5)).flatMap(x => console.log(x)) // x is a promise for 5
Promise.accept(Promise.accept(5)).then(x => console.log(x)) // x is 5

[domenic]

On the other hand, I am not sure how lazy evaluation fits with the current spec either, as we eagerly reach into the promise's current state upon resolving.

In other words, it seems to me that even one-level unwrapping breaks any lazy evaluation techniques, and if we wanted to support lazy evaluation, we should instead allow resolve to do no unwrapping, store the promise as the internal value, and let then do all the unwrapping (either one-level or multi-level). As you've noted in the past though, this is memory-inefficient for long promise chains.

[erights]

When you say "Resolve" in this proposal, I think you're focusing on the implications for the Promise.resolve API, whereas I'm focusing on the implications for the result processing of .then. Specifically, https://github.com/domenic/promises-unwrapping#callhandler--derivedpromise--handler--argument- CallHandler, step iii calls the internal Resolve operation. If result.[[value]] is a promise, the already returned promise must adopt that promise without calling the .then method of the result.[[value]] promise. Otherwise, if result.[[value]] is a non-promise, even if it is a thenable, the already returned promise must accept that value.

Likewise with .flatMap. If the callback returns a promise, the promise already returned by .flatMap must adopt that promise without calling its .flatMap method. Otherwise if result.[[value]] is a non-promise, even if it is a thenable, the already returned promise must be rejected.

[erights]

So if you are really only concerned with changing the spec of the Promise.resolve API and not the semantics of the internal calls to Resolve, that's much more plausible, though I remain skeptical even of that.

[domenic]

No, it is important for the result-processing of .then as well. As @wycats and I point out, returning promise subclasses right now is pretty broken, since any customized then behavior is ignored.

If result.[[value]] is a promise, the already returned promise must adopt that promise without calling the .then method of the result.[[value]] promise.

Why is this a must? Why can't it adopt the promise by using its .then method? That is, schedule a microtask to do (roughly, skipping the get-then-call-then dance)

.then(v => { set already-returned promise's [[Value]] to v },
      r => { set already-returned promise's [[Reason]] to r });

Likewise with .flatMap. If the callback returns a promise, the promise already returned by .flatMap must adopt that promise without calling its .flatMap method.

Similarly I do not see why you must not call the .flatMap method. This would again break subclasses that customize flatMap.

[erights]

Let's start with the example at https://mail.mozilla.org/pipermail/es-discuss/2013-August/032901.html If you adopt by invoking hr.then, you'd force the lazy evaluation to happen when the "v => hr" callback returns, which is way too early.

Since hr.then does assimilation on the input side, we would no longer be postponing assimilation until p2.then is called. It would essentially be a regression to the Promises/A+ behavior before the AP2 revolution.

[domenic]

Right, so it comes down to the laziness question. Small correction:

In current Promises/A+, hr is forced when p2 is constructed, which probably doesn't break the code but is wasteful -- it doesn't need to be forced yet.

Technically hr is forced when p1 is fulfilled, not when p2 is constructed.

But regardless...

It seems very strange that only thenables are allowed to be lazy, and not promises. It seems there is a disconnect, and arguments that work for either should work for both:

• Do one-level unwrapping of promises to preserve memory should imply do one-level unwrapping of thenables to preserve memory.
• Do not do one-level unwrapping of thenables to allow laziness should imply do not do one-level unwrapping of promises to allow laziness.

But from what I can see you are arguing for one-level unwrapping of promises but no unwrapping of thenables, which means promise chains are memory efficient but cannot be lazy, whereas thenable chains are lazy but cannot be memory efficient.

[erights]

Whether promises can be lazy depends on what extension hooks we eventually provide. The Q.makeRemote hook stated at http://wiki.ecmascript.org/doku.php?id=strawman:concurrency#fundamental_static_q_methods and implemented at https://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js#525 does allow lazy promises. The current proposal does not include such hooks, but we should definitely consider them for ES7. You can't do distributed promises and promise pipelining without something like this.

Conjecture: Any extension hooks adequate to support distributed promises with promise pipelining will also be adequate to support lazy promises. I believe this to be true, and is certainly true for all such extension hooks I've looked at.

In any case, for ES6 and DOM before we have such hooks, your statement

But from what I can see you are arguing for one-level unwrapping of promises but no unwrapping of thenables, which means promise chains are memory efficient but cannot be lazy, whereas thenable chains are lazy but cannot be memory efficient.

is correct and gives us the best of both worlds.

[domenic]

Well, that leaves us in the unfortunate place of having to choose between subclassing and future support for laziness. IMO subclassing wins, especially since we don't know what form the laziness hooks will take and they could potentially be done in a way that does not depend on lazily executing then. (That is, then may not be the perform-computation trigger, as it is in the thenable example you link to.)

Perhaps we should flesh these hooks out, and figure out a way for them to be made to work while still being compatible with subclassing.

---

The other possibility is to remove one-level unwrapping of promises, i.e. replace resolve with accept (but keep the name of course). The memory should still be reclaimable via optimizations, e.g. if it is detected that the promise being adopted is a true promise and does not have an overriden then, the implementation can reach inside of that promise and adopt its internal state directly, and thus avoid holding on to the adopted promise.

This will not be observable in a world with only then, although introducing flatMap would change that, meaning if implementations introduce flatMap they would have to remove that optimization.

[erights]

Including .flatMap was essential to achieving TC39 consensus, as you'll remember from the meeting.

Even without .flatMap, the optimization you talk about is observable, and so is not just an optimization, since it accelerates assimilation to when v => mr returned as opposed to when p2.then is called. Since it is observable, the spec has to account for its semantics. This would reintroduce an IsPromise test or its moral equivalent.

[domenic]

Certainly, being flatMap-future-friendly is importantessential.

Even without .flatMap, the optimization you talk about is observable

I do not see how this is possible. If the adopted promise is a true-promise without an overriden .then method, as specified, there should be no way to determine whether it was assimilated at resolve time or at .then time.

[allenwb]

Let me know if I can be of any help with adapting the design for subclassing/extensibility. I'd hate to rush this into ES6 in a manner that isn't subclass extensible and then find out selves painted into a compatibility corner that prevents us from fixing it.

Subclass extensibility is usually a matter of finding an appropriate abstract algorithm that decomposes into a set of over-ridable (ie, extensible) kernel operation. Unfortunately I've only had the time to peripherally follow the promises/thenables design issues so I'm not currently update to speed on the subtleties you are currently discussing.

[wycats]

@allenwb Yep. My claim, at the moment, is that then is a sufficient algorithm for subclassability. Mark wants to use internal state that isn't itself available to subclassing for important parts of the algorithm, which breaks subclassing.

I would also be comfortable with a larger kernel, if it was shown to be needed (probably via symbols), but I am deeply uncomfortable with important parts of the cross-object algorithm requiring internal state, which breaks subclassing.

[domenic]

Fixed by 002ec7a25a97c70eb131096bd3f39da26c6d87c4 and subsequent commits.