tc39 / proposal-cancelable-promises

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

Don't use .promise.then for subscription #59

Closed bergus closed 7 years ago

bergus commented 7 years ago

There is no point in exposing a .promise on the token:

I guess I could find more. What are the advantages of using a promise as part of a token? They allowed for quick prototyping (no need to reimplement all the subscription stuff) and they demonstrated the proposed semantics in a way everyone would be familiar with. I can't think of anything else. So - good for a draft, but nothing mature enough for the language.

My suggestion therefore is to remove the getter

get promise() {
    return this.[[CancelTokenPromise]]
}

and replace it by a method

subscribe(handler) {
    return this.[[CancelTokenPromise]].then(handler)
}

Maybe the promise should be removed altogether even from the internals of CancelTokens, a [[CancelReason]] field of type Option<Cancel<T>> and a [[CancelReactions]] field of type List<Function> should suffice.

domenic commented 7 years ago

Thanks for your input, but the current direction is we're going with for cancel token subscriptions. We're not going to add another developer-facing subscription protocol to the language for single-occurrence events, even if it just wraps the promise as you describe.

bergus commented 7 years ago

There is no "other" subscription protocol - it behaves the same as then. If you want, you can even call it then instead of subscribe to make tokens become thenables. And you already got Promise.withCancelToken as another (superior, actually) method of subscribing to tokens. I would be fine with subscribe being dropped altogether as well.

TBH, closing this issue without even trying to counter my arguments looks a lot like "Yeah, I can see this is the wrong direction but it's the one we decided on". Not what I expected of TC39.

benjamingr commented 7 years ago

I'm in favor of keeping .promise it has been very useful when writing code with tokens. The fact it never rejects is as relevant as the fact Math.max never throws.

The engine can instantiate the object lazily if it needs to.

bergus commented 7 years ago

@benjamingr Can you post (or point to) such an example where .promise is useful? I can't imagine one where Promise.withCancelToken or a token.subscribe would not be a better fit.

benjamingr commented 7 years ago

I'm afk on mobile for a while and in Japan, I can post some examples on Oct 23rd but I posted some motivating examples earlier in this repo that use .promise

bergus commented 7 years ago

I might be missing something, but all the good examples I remember on this repo that used .promise used it only for a .promise.then(…) call. Looking forward for some good reason to keep the promise, thanks!

ljharb commented 7 years ago

Any place I want to resolve another promise with it, or pass it into a function - a promise is more powerful than a callback-taking API precisely because it's composable and I don't have to have the callback ready yet.

bergus commented 7 years ago

@ljharb I don't think there are any common use cases where you'd want to pass a token's promise into a function instead of passing the token itself. If such use cases existed, making the token thenable could take care of them. And don't forget that even subscribe is still a promise-returning API - if you desperately needed a promise, you could just call token.subscribe() (without arguments) and get one.