Promise finally

2018-02-25 Thread Raul-Sebastian Mihăilă
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?

```js
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
```
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-25 Thread Raul-Sebastian Mihăilă
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).
https://github.com/tc39/ecma262/pull/1118/files

On Sun, Feb 25, 2018 at 5:27 AM, Jordan Harband  wrote:

> 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)
>
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-24 Thread Jordan Harband
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)

On Sat, Feb 24, 2018 at 1:22 AM, Raul-Sebastian Mihăilă <
raul.miha...@gmail.com> wrote:

> 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.[[Promise
> Result]])
>   Else
> return FulfillPromise(promise, finallySourcePromise.[[Promise
> Result]])
>   update step 12: Perform EnqueueJob("PromiseJobs",
> PromiseResolveThenableJob, « promise, resolution, thenAction,
> F.[[FinallySourcePromise]] »).
> ```
>
> ___
> es-discuss mailing list
> es-discuss@mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Promise finally

2018-02-24 Thread Raul-Sebastian Mihăilă
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]] »).
```
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Promise finally

2018-02-24 Thread Raul-Sebastian Mihăilă
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]] »).
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Ben Newman
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:
https://github.com/tc39/proposal-promise-finally/pull/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

On Fri, Feb 23, 2018 at 10:19 AM, Raul-Sebastian Mihăilă <
raul.miha...@gmail.com> wrote:

> In other words
>
> ```js
> Promise.resolve().finally(() => {}).then(() => { console.log(1); });
> Promise.resolve().then(() => {}).then(() => { console.log(2); }).then(()
> => { console.log(3); });
> ```
>
> prints 2, then 3, then 1.
>
> ___
> es-discuss mailing list
> es-discuss@mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Promise finally

2018-02-23 Thread Raul-Sebastian Mihăilă
In other words

```js
Promise.resolve().finally(() => {}).then(() => { console.log(1); });
Promise.resolve().then(() => {}).then(() => { console.log(2); }).then(() =>
{ console.log(3); });
```

prints 2, then 3, then 1.
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Promise finally

2018-02-23 Thread Raul-Sebastian Mihăilă
@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.
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Ben Newman
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
, followed
by `Invoke(promise, "then", « ... »)` here
 (and here
).

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

On Fri, Feb 23, 2018 at 9:32 AM, Boris Zbarsky  wrote:

> 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.
>
> -Boris
>
> ___
> es-discuss mailing list
> es-discuss@mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Boris Zbarsky

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.


-Boris
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Michael Luder-Rosefield
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.

On Fri, 23 Feb 2018 at 14:24 Raul-Sebastian Mihăilă 
wrote:

> 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?
>
> On Fri, Feb 23, 2018 at 3:59 PM, Viktor Kronvall <
> viktor.kronv...@gmail.com> wrote:
>
>> 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ă :
>>
>>> I find it weird that
>>>
>>> ```js
>>> 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
>>>
>>> ```js
>>> Promise.resolve().finally().then(() => { console.log(1); });
>>> Promise.resolve().then().then(() => { console.log(2); });
>>> ```
>>>
>>> prints 1 and then 2.
>>> ___
>>> es-discuss mailing list
>>> es-discuss@mozilla.org
>>> https://mail.mozilla.org/listinfo/es-discuss
>>>
>>
> ___
> es-discuss mailing list
> es-discuss@mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Raul-Sebastian Mihăilă
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?

On Fri, Feb 23, 2018 at 3:59 PM, Viktor Kronvall 
wrote:

> 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ă :
>
>> I find it weird that
>>
>> ```js
>> 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
>>
>> ```js
>> Promise.resolve().finally().then(() => { console.log(1); });
>> Promise.resolve().then().then(() => { console.log(2); });
>> ```
>>
>> prints 1 and then 2.
>> ___
>> es-discuss mailing list
>> es-discuss@mozilla.org
>> https://mail.mozilla.org/listinfo/es-discuss
>>
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Re: Promise finally

2018-02-23 Thread Viktor Kronvall
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ă :

> I find it weird that
>
> ```js
> 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
>
> ```js
> Promise.resolve().finally().then(() => { console.log(1); });
> Promise.resolve().then().then(() => { console.log(2); });
> ```
>
> prints 1 and then 2.
> ___
> es-discuss mailing list
> es-discuss@mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss


Promise finally

2018-02-23 Thread Raul-Sebastian Mihăilă
I find it weird that

```js
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

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

prints 1 and then 2.
___
es-discuss mailing list
es-discuss@mozilla.org
https://mail.mozilla.org/listinfo/es-discuss