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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> >> 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 <[email protected]> >> >> > 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 <[email protected]> >> >> >> 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 <[email protected]> >> >> >> > 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 >> >> >> >> [email protected] >> >> >> >> https://mail.mozilla.org/listinfo/es-discuss _______________________________________________ es-discuss mailing list [email protected] https://mail.mozilla.org/listinfo/es-discuss
