Promise finally

# Raul-Sebastian Mihăilă (10 months ago)

I find it weird that

Promise.resolve().finally(() => {}).then(() => { console.log(1); });

Promise.resolve().then(() => {}).then(() => { console.log(2); });

prints 2 and then 1. It would have been possible to spec it in such a way that it would have printed 1 and 2.

On the other hand

Promise.resolve().finally().then(() => { console.log(1); });

Promise.resolve().then().then(() => { console.log(2); });

prints 1 and then 2.

# Viktor Kronvall (10 months ago)

Since these two Promises aren't chained to one another I wouldn't expect any specific deterministic ordering between the console.log statements. Are you suggesting that such a deterministic ordering should be imposed by using micro tasks or what are you proposing here exactly?

In other words, why exactly do you expect the result to always be printing 1 before printing 2?

2018年2月23日(金) 19:21 Raul-Sebastian Mihăilă <raul.mihaila at gmail.com>:

# Raul-Sebastian Mihăilă (10 months ago)

The order is deterministic, as specified, I just don't think it's the right order. I don't have a concrete example with finally, but if I were to imagine one, say you're writing some tests with jest and you want to make some checks in the then callbacks. In order for those checks to be executed in good time, you must return a promise from the test callback. If you have more promises you have to do a Promise.all in order to make sure that you wait for all the promises. If you are able to determine the order in which the promises are settled, you can return the one that is settled the last. This is perhaps not a convincing example, but if this didn't matter why is the order specified?

# Michael Luder-Rosefield (10 months ago)

Whenever you chain a promise with a then/finally, you're basically letting the runtime look at the callbacks at some arbitrary point in the future, no? So despite being written in a defined order, they will be run in whatever order eventuates.

# Boris Zbarsky (10 months ago)

On 2/23/18 9:30 AM, Michael Luder-Rosefield wrote:

Whenever you chain a promise with a then/finally, you're basically letting the runtime look at the callbacks at some arbitrary point in the future, no?

Not if the promise is already known to be resolved. In that case, the exact behavior of the runtime is very clearly specified.

# Ben Newman (10 months ago)

This ordering of console.log calls seems to happen because Promise.prototype.finally is specified in terms of Promise.prototype.then, and is required to call .then twice.

Note the Invoke(promise, "then", « thenFinally, catchFinally ») here tc39.github.io/ecma262/#sec-promise.prototype.finally, followed

by Invoke(promise, "then", « ... ») here tc39.github.io/ecma262/#sec-thenfinallyfunctions (and here tc39.github.io/ecma262/#sec-catchfinallyfunctions).

Any way you cut it, this adds an extra asynchronous tick/step/hop/skip/jump, compared to merely calling Promise.prototype.then.

Implementing Promise extensions in terms of Promise.prototype.then is a good idea, because it means we don't have to add new fundamental operations to the core Promise implementation. However, the translation from .finally to .then comes at a cost. Once you understand that tradeoff, hopefully this behavior seems more reasonable.

Ben

His errors are volitional and are the portals of discovery. -- James Joyce

# Raul-Sebastian Mihăilă (10 months ago)

@Ben Newman There are 2 extra ticks, not just 1. The first one is caused by step 8 of 25.6.5.3.1 and the other one is caused by the fact that the thenFinally callback passed in step 7 of 25.6.5.3 returns a promise. I'm wondering if this trade-off is the right one.

# Raul-Sebastian Mihăilă (10 months ago)

In other words

Promise.resolve().finally(() => {}).then(() => { console.log(1); });

Promise.resolve().then(() => {}).then(() => { console.log(2); }).then(() =>
{ console.log(3); });

prints 2, then 3, then 1.

# Ben Newman (10 months ago)

Yes, indeed, I should have said "is required to call .then at least twice."

It's funny you should mention this nuance, because I recently opened a pull request against the Promise.prototype.finally proposal repository that would solve exactly this problem, as well as simplifying polyfills by removing the need for species constructor logic: tc39/proposal-promise-finally#48

I'm fully aware that my PR came too late to affect the most recent edition of the spec, but perhaps it's not too late to change this behavior in the next edition. I would be willing to champion this refinement to the Promise.prototype.finally spec text, if TC39 is open to creating fewer promise objects and calling .then fewer times, at the expense of altering observable spec semantics (only slightly, I would argue, but observably).

Ben

Ben

His errors are volitional and are the portals of discovery. -- James Joyce

# Raul-Sebastian Mihăilă (10 months ago)

If TC39 finds a better solution than the existing one in good time, I don't think it makes sense for it to wait for another year to implement it while having a broken Promise.prototype.finally in the browsers.

I've been thinking about a solution and, if my solution is correct, the changes are very small. In essence, in this solution, Promise.prototype.finally behaves very similar to Promise.prototype.then. The resolve functions associated with a promise capability, if they are obtain using CreateResolvingFunctions, they will have an internal [[FinallySourcePromise]] slot which will be used to resolve the promise created by Pormise.prototype.finally. The rest of the changes consist basically of passing around the value of this slot. This solution is much more intuitive in terms of what finally is expected to do, which also has the good property of not incurring 2 extra ticks (not even 1 extra tick).

CreateResolvingFunctions(promise, finallySourcePromise) update step 3: Let resolve be CreateBuiltinFunction(stepsResolve, « [[Promise]], [[AlreadyResolved]], [[FinallySourcePromise]] »). insert new step 6: Set resolve.[[FinallySourcePromise]] to finallySourcePromise.

GetCapabilitiesExecutor Functions A GetCapabilitiesExecutor function is an anonymous built-in function that has a [[Capability]] and a [[FinallySourcePromise]] internal slots.

NewPromiseCapability(C, finallySourcePromise = undefined) update step 5: Let executor be CreateBuiltinFunction(steps, « [[Capability]], [[FinallySourcePromise]] »). insert new step 7: Set executor.[[FinallySourcePromise]] to finallySourcePromise.

Promise ( executor ) insert step 8: Let finallySourcePromise be undefined. insert setp 9: If executor has a internal slot [[FinallySourcePromise]] Let finallySourcePromise be executor.[[FinallySourcePromise]] update step 8 (which now becomes step 10): Let resolvingFunctions be CreateResolvingFunctions(promise, finallySourcePromise).

PromiseResolveThenableJob(promiseToResolve, thenable, then, finallySourcePromise) The job PromiseResolveThenableJob with parameters promiseToResolve, thenable, then and finallySourcePromise performs the following steps: update step 1: Let resolvingFunctions be CreateResolvingFunctions(promiseToResolve, finallySourcePromise).

Promise.prototype.finally(onFinally) Let promise be the this value. If IsPromise(promise) is false, throw a TypeError exception. Let C be ? SpeciesConstructor(promise, %Promise%). Let resultCapability be ? NewPromiseCapability(C, promise). Return PerformPromiseThen(promise, onFinally, onFinally, resultCapability).

Promise Resolve Functions update step 7: If Type(resolution) is not Object, then Let finallySourcePromise be F.[[FinallySourcePromise]] If finallySourcePromise is undefined return FulfillPromise(promise, resolution). Else If finallySourcePromise.[[PromiseState]] is "rejected" return RejectPromise(promise, finallySourcePromise.[[PromiseResult]]) Else return FulfillPromise(promise, finallySourcePromise.[[PromiseResult]]) update step 12: Perform EnqueueJob("PromiseJobs", PromiseResolveThenableJob, « promise, resolution, thenAction, F.[[FinallySourcePromise]] »).

# Raul-Sebastian Mihăilă (10 months ago)

Trying better formatting for esdiscuss.org.

CreateResolvingFunctions(promise, finallySourcePromise)
  update step 3: Let resolve be CreateBuiltinFunction(stepsResolve, «
[[Promise]], [[AlreadyResolved]], [[FinallySourcePromise]] »).
  insert new step 6: Set resolve.[[FinallySourcePromise]] to
finallySourcePromise.

GetCapabilitiesExecutor Functions
  A GetCapabilitiesExecutor function is an anonymous built-in function that
has a [[Capability]] and a [[FinallySourcePromise]] internal slots.

NewPromiseCapability(C, finallySourcePromise = undefined)
  update step 5: Let executor be CreateBuiltinFunction(steps, «
[[Capability]], [[FinallySourcePromise]] »).
  insert new step 7: Set executor.[[FinallySourcePromise]] to
finallySourcePromise.

Promise ( executor )
  insert step 8: Let finallySourcePromise be undefined.
  insert setp 9: If executor has a internal slot [[FinallySourcePromise]]
    Let finallySourcePromise be executor.[[FinallySourcePromise]]
  update step 8 (which now becomes step 10): Let resolvingFunctions be
CreateResolvingFunctions(promise, finallySourcePromise).

PromiseResolveThenableJob(promiseToResolve, thenable, then,
finallySourcePromise)
The job PromiseResolveThenableJob with parameters promiseToResolve,
thenable, then and finallySourcePromise performs the following steps:
  update step 1: Let resolvingFunctions be
CreateResolvingFunctions(promiseToResolve,
finallySourcePromise).

Promise.prototype.finally(onFinally)
  Let promise be the this value.
  If IsPromise(promise) is false, throw a TypeError exception.
  Let C be ? SpeciesConstructor(promise, %Promise%).
  Let resultCapability be ? NewPromiseCapability(C, promise).
  Return PerformPromiseThen(promise, onFinally, onFinally,
resultCapability).

Promise Resolve Functions
  update step 7: If Type(resolution) is not Object, then
    Let finallySourcePromise be F.[[FinallySourcePromise]]
    If finallySourcePromise is undefined
      return FulfillPromise(promise, resolution).
    Else
      If finallySourcePromise.[[PromiseState]] is "rejected"
        return RejectPromise(promise, finallySourcePromise.[[
PromiseResult]])
      Else
        return FulfillPromise(promise, finallySourcePromise.[[
PromiseResult]])
  update step 12: Perform EnqueueJob("PromiseJobs",
PromiseResolveThenableJob, « promise, resolution, thenAction,
F.[[FinallySourcePromise]] »).
# Jordan Harband (10 months ago)

Simply theorizing about how it might be done - without an actual spec diff (this email might be close but I can't personally reason about it) - isn't going to achieve much, unfortunately.

However, if you'd like to make a PR to the proposal repo, I'd be happy to review it. If it seems possible, and if it passes the test suites, I'd be happy to make the corresponding PR to the actual spec and test262.

(as for "waiting a year", ES is a living standard; there's no need to wait that long. If a change is presented that results in fewer observable calls without violating any of the criteria that led to the current spec, I suspect the committee and implementors would be more than happy to see the change go in ASAP)

# Raul-Sebastian Mihăilă (10 months ago)

I made the PR on the ecma repo so that the diff is smaller (since I'm touching more sections than the proposal repo had). tc39/ecma262/pull/1118/files

# Raul-Sebastian Mihăilă (10 months ago)

This is an illustration of the current Promise.prototype.finally deficiency. In this example, the incr method does 2 things. It increases count by 1. And increases methodCallsCount by 1. At a later point in time, it was decided to add an incr3 method that did the same, but increase count by 3, not by 1. There are two approaches illustrated, the good counter and the bad counter. Promise.prototype.finally is currently the bad counter. The good counter's incr3 does exactly what it was supposed to do, namely increases count by 3 and increases methodCallsCount by 1. The bad counter's incr3 calls incr 3 times in order to increase count by 1 three times, thinking that it's equivalent. The problem is that incr wasn't just increasing count by 1. It did more than that. Therefore incr3 is not expressible in terms of incr. Similarly, Promise.prototype.finally shouldn't call Promise.prototype.then, because, conceptually, it's definition doesn't say that it must incur 2 extra ticks.

It's possible that library and framework authors will avoid using finally for efficiency. The meeting notes don't illustrate that TC39 considered this nuance seriously. Is it possible for TC39 to reconsider this matter?

class GoodCounter {
  constructor() {
    this.count = 0;
    this.methodCallsCount = 0;
  }

  incr() {
    this.count += 1;
    this.methodCallsCount += 1;
  }

  incr3() {
    this.count += 3;
    this.methodCallsCount += 1;
  }
}

class BadCounter {
  constructor() {
    this.count = 0;
    this.methodCallsCount = 0;
  }

  incr() {
    this.count += 1;
    this.methodCallsCount += 1;
  }

  incr3() {
    this.incr();
    this.incr();
    this.incr();
  }
}

const c1 = new GoodCounter();

c1.incr();

c1.count; // 1
c1.methodCallsCount; // 1

c1.incr3();

c1.count; // 4
c1.methodCallsCount; // 2

const c2 = new BadCounter();

c2.incr();

c2.count; // 1
c2.methodCallsCount; // 1

c2.incr3();

c2.count; // 4
c2.methodCallsCount; // 4