tc39 / proposal-cancelable-promises

Former home of the now-withdrawn cancelable promises proposal for JavaScript
Other
376 stars 29 forks source link

`withCancelToken` should not call the executor when already cancelled #54

Closed bergus closed 8 years ago

bergus commented 8 years ago

I think it is pointless to call the executor at all when we already know that the passed-in token had been requested and the promise would be rejected anyway. We should add a step

If cancelToken is not undefined and !CancelTokenIsRequested(cancelToken) is true, return Promise.reject(cancelToken.[[CancelTokenPromise]].[[PromiseResult]])

after step 3 of §1.3.1 Promise.withCancelToken.

This should prevent some unnecessary work and remove the need to put a token.throwIfRequested() call at the begin of every executor function. The only difference in behaviour is when the executor does synchronously throw, resolve or reject, which currently takes precedence over the immediately scheduled promise job that cancels the promise.

domenic commented 8 years ago

No, the executor is supposed to be always executed, just like in the promise constructor. That is the desired design.

bergus commented 8 years ago

Why is that desired? I would not want to have the executor start some work when I already know that it is cancelled. Yes, I can always write

Promise.withCancelToken(token, resolve => {
    token.throwIfRejected();
    const id = setTimeout(resolve);
    return () => clearTimeout(id);
})

but that first line in the executor feels like boilerplate that should be unnecessary.

domenic commented 8 years ago

For clarity of the programming model among the different ways of creating promises.

bergus commented 8 years ago

I see, I thought withCancelToken was different enough from the regular promise constructor (especially when we consider #53) to warrant this, but maybe I'm biased having implemented both myself.

benjamingr commented 8 years ago

I'm +1 on always executing the executor - not doing so would be really confusing to users.