Promise.finally not really final

Jon Ronnenberg jon.ronnenberg at gmail.com
Fri Sep 7 20:51:30 UTC 2018


You are probably right that it is too late.

> inconsistent with expected behavior because it would have to run multiple times.

Is it inconsistent with expected behavior, if someone adds to the
Promise chain in another tick? Seems like a pretty conscious choice,
and the `finally` code would have to clean up that as well.

I guess my main issue is that I don't really see a use-case for a
`finally` block that runs before a `then` block. It's just a
convenience for not writing the same function twice as a variable.
Yah, ok. I'll live with it and move on. A shame though.

PS. but I just want to ask if you ran my example? ;)


On Fri, Sep 7, 2018 at 10:37 PM Peter Jaszkowiak <p.jaszkow at gmail.com> wrote:
>
> Having finally run after all of them in just the tick is very inconsistent.
>
> Having it run after the end of the chain is either impossible because it would have to wait indefinitely, or inconsistent with expected behavior because it would have to run multiple times.
>
> I think you should accept the specified behavior, it's very very unlikely to change at this point, even if you did have a good argument.
>
> On Fri, Sep 7, 2018, 14:27 Jon Ronnenberg <jon.ronnenberg at gmail.com> wrote:
>>
>> Hmm, so I misunderstand finally then. Running your code with `then`
>> gives NaN. x is undefined - I just ran it in node 8.11
>>
>> let b = Promise.resolve(4).then(() => console.log('done'));
>> setTimeout(() => b.then(x => console.log(x + 3)), 1000);
>>
>> But running it in Firefox 63.0b4 gives `done` -> `7`
>>
>> let b = Promise.resolve(4).finally(() => console.log('done'));
>> setTimeout(() => b.then(x => console.log(x + 3)), 1000);
>>
>> I would still prefer that finally was run either after the last
>> then/catch in the same "tick" or after the last then/catch for the
>> duration of the promise lifetime.
>>
>> So that:
>> let b = Promise.resolve(4).finally(() => console.log('done')).then((x) => {
>> console.log('another function added this later')
>> return x
>> });
>> setTimeout(() => b.then(x => console.log(x + 3)), 1000);
>>
>> Would give "another function added this later" -> "done" -> 7 -> "done"
>> And not as it is currently: "done" -> "another function added this later" -> 7
>>
>> I made  an lengthy example here just to see if it was possible (it
>> runs in node 8.11):
>>
>> function Promise(executor) {
>>   if(executor == undefined) throw new TypeError('Not enough arguments
>> to Promise.');
>>
>>   let thenables = [];
>>   let rejects = [];
>>   let catches = [];
>>   let finallies = [];
>>   let states = {
>>     pending: 1,
>>     fulfilled: 2,
>>     rejected: 4
>>   }
>>   let state = states.pending;
>>   let value;
>>   let timer;
>>
>>   function runFinallies () {
>>     finallies.forEach(f => {
>>       try {
>>         f()
>>       } catch (e) { /* we just skip throwing finallies since our catch
>> block has already executed */ }
>>     })
>>   }
>>
>>   this.then = (f, r) => {
>>     if(timer)
>>       timer = clearTimeout(timer);
>>     thenables.push(f);
>>     rejects.push(r);
>>
>>     if(state === states.fulfilled)
>>       timer = setTimeout(this.resolve.bind(this, value), 0);
>>     else
>>     if(state === states.rejected)
>>       timer = setTimeout(this.reject.bind(this, value), 0);
>>
>>     return this;
>>   };
>>   this.resolve = v => {
>>     state = states.fulfilled;
>>     value = v;
>>
>>     let chainableValue = value;
>>     thenables.forEach(f => {
>>       try {
>>         chainableValue = f(chainableValue || value)
>>       } catch(e) {
>>         if(catches.length === 0)
>>           this.reject(e);
>>         else {
>>           catches.forEach(f => f(e));
>>           catches = [];
>>         }
>>       } finally {
>>         runFinallies()
>>       }
>>     });
>>     thenables = [];
>>   };
>>   this.reject = v => {
>>     state = states.rejected;
>>     value = v;
>>
>>     let chainableValue = value;
>>     let error;
>>     try {
>>       rejects.forEach(f => chainableValue = f(chainableValue || value));
>>     } catch (e) {
>>       error = e;
>>     } finally {
>>       runFinallies()
>>     }
>>     rejects = [];
>>     if(error) throw error;
>>     // console.log(this, state, value, rejects);
>>   };
>>   this.catch = f => {
>>     catches.push(f);
>>     return this;
>>   };
>>   this.finally = f => {
>>     finallies.push(f);
>>     return this;
>>   };
>>
>>   try {
>>     executor(this.resolve, this.reject);
>>   } catch(e) {
>>     if(state !== states.fulfilled)
>>       this.reject(e);
>>   }
>> }
>> Promise.resolve = function(v) {
>>   let p;
>>   if(v instanceof Promise)
>>     p = v;
>>   else if(v.then)
>>     p = new Promise(v.then);
>>   else
>>     p = new Promise(resolve => resolve(v));
>>   return p;
>> }
>> Promise.reject = function(v) {
>>   return new Promise((resolve, reject) => reject(v));
>> }
>> Promise.race = function(iterable) {
>>   return new Promise((resolve, reject) => {
>>     let muteResolve = false,
>>         muteReject = false;
>>
>>     iterable.forEach(promise => {
>>       promise.then(
>>         value => {
>>           muteReject = true;
>>           if(!muteResolve)
>>             resolve(value);
>>         },
>>         value => {
>>           muteResolve = true;
>>           if(!muteReject)
>>             reject(value)
>>         }
>>       )
>>     })
>>   });
>> };
>> Promise.all = function(iterable) {
>>   // let allResolved = false;
>>   let resolvedLength = iterable.length;
>>   let promiseValues = [];
>>   return new Promise((resolve, reject) => {
>>     iterable.map(
>>       promise => promise instanceof Promise ?
>>         promise : Promise.resolve(promise)
>>     ).forEach(promise => promise.then(
>>       value => {
>>         if(resolvedLength === 1)
>>           resolve(promiseValues.concat(value));
>>         else {
>>           resolvedLength--;
>>           promiseValues.push(value);
>>         }
>>       },
>>       value => reject(value)
>>     ));
>>   });
>> }
>>
>> let b = Promise.resolve(4).finally(() => console.log('done')).then((x) => {
>>   console.log('another function added this later')
>>   return x
>> });
>> setTimeout(() => b.then(x => console.log(x + 3)), 1000);
>>
>>
>> On Fri, Sep 7, 2018 at 9:49 PM Peter Jaszkowiak <p.jaszkow at gmail.com> wrote:
>> >
>> > I think you're the one who's confused. Chains exist within a tick, and persist beyond a single tick.
>> >
>> > ```
>> > let b = Promise.resolve(4).finally(() => console log('done'));
>> >
>> > setTimeout(1000, () => b.then(x => console.log(x + 3)));
>> > ```
>> >
>> > There is no way to know that the latter then exists until it is executed. There's no way for the finally to execute after it without waiting a second.
>> >
>> > The only thing you could possibly specify is that the finally must execute after all thens added within the same tick. That will never happen, though, because it would just be inconsistent behavior.
>> >
>> > On Fri, Sep 7, 2018, 13:40 Jon Ronnenberg <jon.ronnenberg at gmail.com> wrote:
>> >>
>> >> Sorry but that is not how Promises work. You compose the entire
>> >> promise chain in one "tick" and then you start execution of the entire
>> >> chain.
>> >>
>> >> I will try to write you an over simplification of the concept... Hang on :)
>> >> On Fri, Sep 7, 2018 at 9:35 PM Peter Jaszkowiak <p.jaszkow at gmail.com> wrote:
>> >> >
>> >> > No, it doesn't make any sense.
>> >> >
>> >> > Do you not see the impossibility of this? If you have a promise `a`:
>> >> >
>> >> > You call `b = a.then(one).finally(done)`
>> >> >
>> >> > `b` is a promise. You can't have the thread waiting forever for every possible descendent to be added before executing the `done` function. There may even be none.
>> >> >
>> >> > On Fri, Sep 7, 2018, 13:27 Jon Ronnenberg <jon.ronnenberg at gmail.com> wrote:
>> >> >>
>> >> >> I'm suggesting that finally is the last function to be executed. So
>> >> >> the chain is, then().then().then().catch().finally(), regardless of at
>> >> >> what time then or catch is added to the Promise chain.
>> >> >> Like try->catch->finally with a callback but  without a callback. Does
>> >> >> it make sense?
>> >> >> On Fri, Sep 7, 2018 at 9:24 PM Peter Jaszkowiak <p.jaszkow at gmail.com> wrote:
>> >> >> >
>> >> >> > There are plenty of ways of doing what you need without changing the behavior of finally. Are you suggesting that calling finally make it so any other then or catch call just not execute? And are you also suggesting that this should climb up the chain of promises?
>> >> >> >
>> >> >> > On Fri, Sep 7, 2018, 13:16 Jon Ronnenberg <jon.ronnenberg at gmail.com> wrote:
>> >> >> >>
>> >> >> >> I know I am late to the game and that Promise.prototype.finally is already in stage 4 but(!).
>> >> >> >>
>> >> >> >> It's just not very useful to have a final function when it's not the final function to run. If it's suppose to be for cleanup, then the current implementation is seriously lacking usefulness.
>> >> >> >>
>> >> >> >> Consider the following example:
>> >> >> >>
>> >> >> >> <audio
>> >> >> >>   class="i-am-the-element"
>> >> >> >>   autoplay="autoplay"
>> >> >> >>   controls="controls">
>> >> >> >>     <source type="audio/mp3" src="http:\/\/play.dogmazic.net\/play\/index.php?type=song&oid=22951&uid=-1&name=Black%20poetry%20-%20Aime-.mp3">
>> >> >> >> </audio>
>> >> >> >> <script>
>> >> >> >>   class Demo {
>> >> >> >>     constructor (element) {
>> >> >> >>       this.node = element
>> >> >> >>     }
>> >> >> >>     destroy () {
>> >> >> >>       return new Promise(resolve => {
>> >> >> >>         // do something or nothing
>> >> >> >>         resolve()
>> >> >> >>       }).finally(() => {
>> >> >> >>         // schedule for DOM removal
>> >> >> >>         this.node = null
>> >> >> >>       })
>> >> >> >>     }
>> >> >> >>   }
>> >> >> >>
>> >> >> >>   const demo = new Demo(document.querySelector('.i-am-the-element'))
>> >> >> >>
>> >> >> >>   setTimeout(() => {
>> >> >> >>     demo.destroy().then(() => {
>> >> >> >>    // will throw an error because finally was run before
>> >> >> >>       demo.node.pause()
>> >> >> >>     }).catch(console.error)
>> >> >> >>   }, 3000)
>> >> >> >> </script>
>> >> >> >>
>> >> >> >> One grand idea about promises is to delegate and compose asynchronous functions, but the current implementation can not be used to for code delegation.
>> >> >> >>
>> >> >> >> From the top of my head the only way to have consumer code,  tap into an execution process is to use callbacks which is what Promises were suppose to help alleviate.
>> >> >> >>
>> >> >> >> <audio
>> >> >> >>   class="i-am-the-element"
>> >> >> >>   autoplay="autoplay"
>> >> >> >>   controls="controls">
>> >> >> >>     <source type="audio/mp3" src="http:\/\/play.dogmazic.net\/play\/index.php?type=song&oid=22951&uid=-1&name=Black%20poetry%20-%20Aime-.mp3">
>> >> >> >> </audio>
>> >> >> >> <script>
>> >> >> >>   class Demo {
>> >> >> >>     constructor (element) {
>> >> >> >>       this.node = element
>> >> >> >>     }
>> >> >> >>     destroy (callback) {
>> >> >> >>       // do something or nothing
>> >> >> >>       try {
>> >> >> >>         callback()
>> >> >> >>       } finally {
>> >> >> >>         // schedule for DOM removal
>> >> >> >>         this.node = null
>> >> >> >>       }
>> >> >> >>     }
>> >> >> >>   }
>> >> >> >>
>> >> >> >>   const demo = new Demo(document.querySelector('.i-am-the-element'))
>> >> >> >>
>> >> >> >>   setTimeout(() => {
>> >> >> >>     demo.destroy(() => {
>> >> >> >>       demo.node.pause()
>> >> >> >>     })
>> >> >> >>   }, 3000)
>> >> >> >> </script>
>> >> >> >>
>> >> >> >> If at all possible, please amend to the spec before it's too late! ... or just drop it.
>> >> >> >>
>> >> >> >> My current use-case is that I work with PSPDFKit and can not get DOM access but rather schedule removal of DOM nodes via their API, but I can pause audio/video - just not using Promise.prototype.finally as it is currently envisioned.
>> >> >> >>
>> >> >> >> Regards, Jon
>> >> >> >>
>> >> >> >> PS. Tested in Firefox 63.0b3 and Safari 11.1.2
>> >> >> >> Here is a polyfill if you need: https://cdn.polyfill.io/v2/polyfill.minify.js?features=Promise.prototype.finally&flags=gated
>> >> >> >>
>> >> >> >> _______________________________________________
>> >> >> >> es-discuss mailing list
>> >> >> >> es-discuss at mozilla.org
>> >> >> >> https://mail.mozilla.org/listinfo/es-discuss


More information about the es-discuss mailing list