Promise.finally not really final

# Jon Ronnenberg (6 years ago)

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="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 ,

t a p into a n 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="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.

, Jon

PS. Tested in Firefox 63.0b3 and Safari 11.1.2 Here is a polyfill if you need: cdn.polyfill.io/v2/polyfill.minify.js?features=Promise.prototype.finally&flags=gated

# Jon Ronnenberg (6 years ago)

Sorry for re-posting but wanted to send this plain-text and didn't figure out that "remove formatting" in gmail wasn't it. I hope everyone can read this now :)

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="play.dogmazic.net/play/index.php?type=song&oid=22951&uid=-1&name=Black poetry - Aime-.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="play.dogmazic.net/play/index.php?type=song&oid=22951&uid=-1&name=Black poetry - Aime-.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.

, Jon

PS. Tested in Firefox 63.0b3 and Safari 11.1.2 Here is a polyfill if you need: cdn.polyfill.io/v2/polyfill.minify.js?features=Promise.prototype.finally&flags=gated

# Peter Jaszkowiak (6 years ago)

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?

# Jon Ronnenberg (6 years ago)

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="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="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.

, Jon

PS. Tested in Firefox 63.0b3 and Safari 11.1.2 Here is a polyfill if you need: cdn.polyfill.io/v2/polyfill.

minify.js?features=Promise.prototype.finally&flags=gated

# Peter Jaszkowiak (6 years ago)

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.

# Jon Ronnenberg (6 years ago)

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 :)

# Peter Jaszkowiak (6 years ago)

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.

# Jon Ronnenberg (6 years ago)

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);

# Peter Jaszkowiak (6 years ago)

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.

# Jon Ronnenberg (6 years ago)

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? ;)

# Isiah Meadows (6 years ago)

If this helps, finally is like finally in try/finally.

# Augusto Moura (6 years ago)

Why would you destroy a node and .then() pause it? It doesn't make sense even in plain English. If the contract of the method (expected behavior) is to destroy the internal node, using it after is a error. You can easily reimplement your use case reversing the logic:

setTimeout(() => {
  demo.node.pause();
  demo.destroy();
}, 3000);

Another solution is to implement a beforeDestroy event emitter and listen to it, this way is certain that the code will be executed always before the node is gone and after any async destroy logic:

class Demo {
  constructor(el) {
    this.beforeDestroy = new EventEmitter();
    // Using the @angular/core EventEmitter because is more "close to English"
    this.node = el;
  }
  destroy() {
    return new Promise(resolve => resolve())
      .then(() => this.beforeDestroy.emit())
      .finally(() => {
        this.node = null;
      });
  }
}

const demo = new Demo(document.querySelector('video'));

demo.beforeDestroy.subscribe(() => {
  demo.node.pause();
});

setTimeout(() => {
  demo.destroy();
}, 3000);

Anyway, the then, catch and finally methods mimic the serial try/catch/finally, simply doesn't make sense finally statements protecting code beyond it's try block definition, and chaining new .thens are beyond the promise definition. Also, async functions already has finally blocks implementations in the same way as the current .finally method spec, implementing a different behaviour would be unnecessarily confusing.

# Jon Ronnenberg (6 years ago)

Why would you destroy a node and .then() pause it? It doesn't make

sense even in plain English. If the contract of the method (expected behavior) is to destroy the internal node, using it after is a error.

The use case is a situation I am currently in, where I have a destroy function that is the same for 10 classes and I can not remove their DOM nodes myself because they are in an iframe and I'm coding against a black box API (proprietary).

Sometimes the removal fail or the DOM node is not removed due to performance optimization, not sure but my code needs to run regardless and updates to their API will happen long after I leave this project.

The symptom is that I can still hear the sound of video/audio, after it should have been removed. A few classes dealing with video and audio could add a this.node.pause() to the promise chain without changing the destroy trait or any other non audible dealing class.

There is many ways to implement this and I thought Promises would be an elegant implementation but since finally is called before then, even in the same call stack, a Promise is not useful.

I've never had a use for adding to a promise chain in the next frame, which is the main argument against calling finally at the end, so I might be the wrong audience.

I have demonstrated how it could work and I think it makes better sense but I'm not up for a fight to defend my way.

# Isiah Meadows (6 years ago)

The promise disposer pattern works basically like this:

// Promise chain
function withFoo(init) {
    return getFoo().then(foo =>
        Promise.resolve()
        .then(() => init(foo))
        .finally(() => foo.destroy())
    )
}

// Async function
async function withFoo(init) {
    const foo = await getFoo()
    try {
        return await init(foo)
    } finally {
        await foo.destroy()
    }
}

// Example use
withFoo(async foo => {
    await foo.changeThings()
    return foo.getValue()
})

That's one big place where this is useful.

# Jack Lu (6 years ago)

Adding to a promise chain in the next frame is not uncommon. For instance, you may want to chain a procedure conditionally by some condition calculated against some data in indexeddb, where you cannot complete in the current tick(all indexeddb operations are async). There're more use cases when you are developing a library. It's true that most usages of promises is 'static', but promise is not limited to it. It can be used more dynamically. For your use case, think of your target as to provide a destroy method with part of it injectable. You can just pass the code you want to inject in as a parameter, or give it a field to config. It's not so relevant to whether Promise.finally is really finally, because it cannot be done directly in traditional try..finally, either.

------------------ 原始邮件 ------------------ 发件人: "Jon Ronnenberg"<jon.ronnenberg at gmail.com>; 发送时间: 2018年9月8日(星期六) 上午8:51 收件人: "Augusto Moura"<augusto.borgesm at gmail.com>; 抄送: "es-discuss"<es-discuss at mozilla.org>; 主题: Re: Promise.finally not really final

Why would you destroy a node and .then() pause it? It doesn't make

sense even in plain English. If the contract of the method (expected behavior) is to destroy the internal node, using it after is a error.

The use case is a situation I am currently in, where I have a destroy function that is the same for 10 classes and I can not remove their DOM nodes myself because they are in an iframe and I'm coding against a black box API (proprietary).

Sometimes the removal fail or the DOM node is not removed due to performance optimization, not sure but my code needs to run regardless and updates to their API will happen long after I leave this project.

The symptom is that I can still hear the sound of video/audio, after it should have been removed. A few classes dealing with video and audio could add a this.node.pause() to the promise chain without changing the destroy trait or any other non audible dealing class.

There is many ways to implement this and I thought Promises would be an elegant implementation but since finally is called before then, even in the same call stack, a Promise is not useful.

I've never had a use for adding to a promise chain in the next frame, which is the main argument against calling finally at the end, so I might be the wrong audience.

I have demonstrated how it could work and I think it makes better sense but I'm not up for a fight to defend my way.

On Sat, 8 Sep 2018 01:18 Augusto Moura, <augusto.borgesm at gmail.com> wrote:

Why would you destroy a node and .then() pause it? It doesn't make sense even in plain English. If the contract of the method (expected behavior) is to destroy the internal node, using it after is a error. You can easily reimplement your use case reversing the logic:

setTimeout(() => {
  demo.node.pause();
  demo.destroy();
}, 3000);

Another solution is to implement a beforeDestroy event emitter and listen to it, this way is certain that the code will be executed always before the node is gone and after any async destroy logic:

class Demo {
  constructor(el) {
    this.beforeDestroy = new EventEmitter();
    // Using the @angular/core EventEmitter because is more "close to English"
    this.node = el;
  }
  destroy() {
    return new Promise(resolve => resolve())
      .then(() => this.beforeDestroy.emit())
      .finally(() => {
        this.node = null;
      });
  }
}

const demo = new Demo(document.querySelector('video'));

demo.beforeDestroy.subscribe(() => {
  demo.node.pause();
});

setTimeout(() => {
  demo.destroy();
}, 3000);

Anyway, the then, catch and finally methods mimic the serial try/catch/finally, simply doesn't make sense finally statements protecting code beyond it's try block definition, and chaining new .thens are beyond the promise definition. Also, async functions already has finally blocks implementations in the same way as the current .finally method spec, implementing a different behaviour would be unnecessarily confusing. Em sex, 7 de set de 2018 às 16:16, Jon Ronnenberg <jon.ronnenberg at gmail.com> escreveu:

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="play.dogmazic.net/play/index.php?type=song&oid=22951&uid=-1&name=Black poetry - Aime-.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="play.dogmazic.net/play/index.php?type=song&oid=22951&uid=-1&name=Black poetry - Aime-.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.

, Jon

PS. Tested in Firefox 63.0b3 and Safari 11.1.2 Here is a polyfill if you need: cdn.polyfill.io/v2/polyfill.minify.js?features=Promise.prototype.finally&flags=gated


es-discuss mailing list es-discuss at mozilla.org, mail.mozilla.org/listinfo/es