tc39 / proposal-cancelable-promises

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

Add cancellation handler #36

Closed bergus closed 8 years ago

bergus commented 8 years ago

You said

I do not want to discuss adding a cancel-specific handler, as @nlwillia does in his tome; I think handling cancelations should be rare, and since we cannot do third state any more, it doesn't make sense as a primitive.

and I definitely think this does need to be discussed. After all, letting cancellation propagate downwards suggests that it might need to be handled, is that not so? Distinguishing rejections and throw-values in cancel and non-cancel values does effectively add a third state like you had it before, the only difference is now that catch (both the method and the keyword) do fire on both types.

If you do not think that cancellations need to be handled when propagating downwards, we should consider merging this proposal with mine - I don't care a lot about the representation of a cancelled promise, but my approach does solve some of the issues brought up at the TC meeting. My draft could benefit from the addition of else/except/whatever-we-call-it, making the optional token argument only necessary where you don't trust your promise creator to cancel correctly or when it's not cancellable at all.

domenic commented 8 years ago

You have not given any reasons for why cancelation should be handled specially. In committee, a few were brought up as to why making it easy to do so was a bad idea, as people might inadvertently transform them into errors. You need some very strong reasons why it needs to be syntactically convenient to handle cancelations specifically to overcome the committee's objections to including a special cancel handler, and I don't see any presented here.

As I've told you earlier, I don't have any intentions of working on or merging with your proposal, and I'd appreciate if you kept discussions of that to its own repository.

bergus commented 8 years ago

Actually I don't see much reason for handling cancellation requests if not on the token level. It would but be a symmetric counterpart to the else/except utility, but I don't have a real use case for it myself. There seem to have been requests to add it though, so I thought we'd need to give it some discussion at least.

It also was one of the fundamental distinctions between our approaches, and now it's gone in your new draft… I'm ok with that.

As I've told you earlier, I don't have any intentions of working on or merging with your proposal, and I'd appreciate if you kept discussions of that to its own repository.

Working on my own repo is pointless until there's a champion for it. Given that my approach offers some solutions to the problems your proposal has, I'd think you'd appreciate my input and would give my ideas some thought. If that is unwanted, please tell me so blatantly. Have you even read the files in my repo and the mail I sent to es-discuss with the comparison between the approaches?

domenic commented 8 years ago

Happy to have a discussion, and your input there is definitely appreciated, but please stop derailing such discussions with meta-concerns about your repo.

bergus commented 8 years ago

It's not a purely meta concern, I could contribute much better to your proposal if I knew that you are familar with my approach, what you think of its soundness, and where you are disagreeing with my assessments. Are you?

domenic commented 8 years ago

Final warning.

bergus commented 8 years ago

Back to topic, you were asking for use cases. In #10 (which is essentially this discussion with positions reversed) @inikulin presented this example where he needs a cancellation handler that does not simply perform cleanup like finally would.

And of course there's the end of the chain, where one would naturally be interested in the resolution that the promise took. Let's make a contrived example:

function loadAndShow(url, token) {
    if (token.requested) return;
    const w = openWindow();
    w.display(spinner);
    load(url, token)
    .finally(() => w.remove(spinner))
    .then(res => {
        w.display(res);
    }, err => {
        if (Cancel.isCancel(err)) {
            w.close();
        } else {
            w.display(String(err));
        }
    });
}

If not using the power-user then with two callbacks, most people would probably prefer to write it as

.then(w.display).onCancel(w.close).catch(w.display)
// or
.then(w.display).except(w.display).onCancel(w.close)

And then there is of course this "simple" idea:

.then(w.display).except(w.display);
token.promise.then(w.close);

But that doesn't really work like we want it to (or at least, not like the above solutions).

To prevent the first and last case where load is "misbehaving", we'd basically need to use Promise.race([token.promise, load(url, token)]). To prevent the second case, we'd need to use some kind of flag that load completed and that the window should not be closed - effectively unsubscribing the cancel callback from the token.

domenic commented 8 years ago

I've decided to not include this for now as part of the initial proposal. Including such a clause got strong concern from TC39, as mentioned above. We should see how the ecosystem reacts to cancelation, and whether this is important enough to be afforded syntactic convenience to override those concerns. In the meantime, there are a number of workarounds which people can use:

The latter of these should also be fairly easy to experiment with in transpilers, so if people build a Babel plugin for catch cancel or similar and it becomes popular, that will definitely provide useful input.

bergus commented 8 years ago

I'm not asking for a new syntactic clause, I'm completely fine with

Checking token.reason in the finally block

but I would like to see a new method for the cases where async/await is not used. Using .catch is ugly as it required rethrowing, and there's no .finally yet in which an if (token.requested) check could be placed.

benjamingr commented 8 years ago

Catching cancellations in catch blocks is very annoying behavior IMO in C#. We're not introducing a 'fixed catch' or saying catch doesn't catch cancellations which is pretty weird.

I'd be content with only being handled in finally and not catch and then cancellations can be handled in the finally via token.requested on the other hand.