tc39 / proposal-cancelable-promises

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

Change Promise.withCancelToken to prioritize the cancel token over resolved-but-pending input promises #53

Closed bergus closed 8 years ago

bergus commented 8 years ago

Just an idea that I think could be useful and is worth discussing: I would make Promise.withCancelToken cancel (i.e. reject) the promise also when resolve has already been called, but it is not yet settled. A promise resolved with a pending promise then they might still get cancelled:

const {token, cancel} = CancelToken.source();
setTimeout(cancel, 1000, "over");
const promise = Promise.withCancelToken(token, resolve => {
    setTimeout(() => {
        resolve(new Promise()); // never resolved, whatever
    }, 500);
});
promise.catch(e => {
    console.log("It's "+e.message);
});

would log "It's over" after one second.

ljharb commented 8 years ago

Once promise A is resolved with promise B, the future of promise A has been handed over to promise B - in other words, promise B's state shouldn't affect the cancellability of A except that when B is cancelled, A is too.

domenic commented 8 years ago

Yes, I don't plan on breaking this invariant of promises.

bergus commented 8 years ago

I don't think this is breaking an invariant - it would just make Promise.withCancelToken behave more like

Promise.race([
    token.promise.then(e => { cancelAction(e); throw e}),
    new Promise(…)
])

than to race for the reject resolving function inside the promise constructor. This could be more desirable, I'd like to get your feedback on it.

domenic commented 8 years ago

I see, the title of this issue was misleading; I thought it was referring to all promises, not just the behavior of Promise.cancelToken. Let's reopen and rename for further discussion.

domenic commented 8 years ago

Although I liked this idea at first, if I am understanding this correctly, I think it will not work. It requires making Promise.withCancelToken too different from the Promise constructor, because it means resolve would no longer act to resolve the being-created promise directly, but would instead resolve to something that races with the cancel token's promise. Maintaining parallel with the Promise constructor is a very high priority here, and making resolve() behave differently would be surprising.

Did I understand correctly?

bergus commented 8 years ago

Yes, you understood that correctly. I would however argue that the difference is not very large, resolve being called with something thenable is an edge case (disregarding the internals of Promise.resolve) anyway. In practical usages of the promise constructor, resolve is always used to fulfill the promise. I guess the behaviour of resolve with thenable objects is already surprising to most promise users. If the idea/documentation of withCancelToken is that the passed in token will reject the returned promise, the behaviour of resolve with thenables is even more surprising since cancellation doesn't work any more.

domenic commented 8 years ago

OK. Given that understanding, I am unwilling to make this change, and am sure it would not get committee consensus. Closing.