New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Request] Improve cancellation API #83
Comments
I had started writing a reply in #68, so a little out of context, but mostly a reply to @cpreston321:
Right, I had similar thoughts but you beat me to it. But this has now been solved and would still be solved by the proposed solution, no?
Here I'm not following. I've heard you mention moving parts of the cancel logic into core a couple of times, but not seen any concrete plan or sign-off from @natemoo-re on this?
I read that they don't want to repeat themselves with handling cancel logic for each prompt , which
Yeah, for sure. Most things are. I don't find it more intuitive, nor better DX. I'll try to explain why in more detail: Apart from being inconsistent with the API of individual prompts*, I prefer having one outlet. I find it confusing and potentially hard to reason about mixing Promises and callbacks. I call an async function and awaits its results. But it might not resolve (or will it?) because we also provide a callback, which might be invoked (or will it?). * having the API of individual prompts follow the group API might be a potential solution, whatever we settle on in the end, to this problem
Thanks for the examples. Always helpful with real-world use cases. I don't know about the last one, but the others sound legit. OK, so I'm totally with you that useful data is lost with the
It would look something along the lines of: const results = await p.group({ ... });
if (isCancelError(results)) {
cancel("Operation cancelled.");
process.exit(0);
}
// or
const { data/results, error } = await p.group({ ... });
if (error instanceof CancelError) {
cancel("Operation cancelled.");
process.exit(0);
}
Maybe I'm reading you wrong, but I think we should be opinionated and not provide multiple ways to handle this. |
I'm not sure I understand at what level you suggest signals are to be introduced? Should the user provide it? AFAIK, signals are a way for the consumer to cancel an async function. But that wouldn't be the case here, would it? |
I'm sure frontend-Twitter has something to say on the matter of signals 😜 |
Did I kill the discussion...? Anyway, sort of related to what I talk about above: https://twitter.com/mattpocockuk/status/1633064377518628866 |
I would like to share my two cents npm Thus, I think it is better if it does not rely on symbols. I feel like this is an instance where symbols are used when they shouldn't be. |
One solution would be using Symbol.for to allow reusable global symbols, this situation could even happen if two (hoisted) versions of clack exist as well. |
Why not just have the |
Is your feature request related to a problem? Please describe.
We've gotten feedback that the
Symbol('clack:cancel')
approach and theonCancel
don't feel like the ideal API for handling cancellation, and I agree.I'm opening this issue to continue the discussion about what an API for graceful cancellation could look like.
Describe the solution you'd like
Seems like
AbortController
is designed specifically for this use case, so maybe there's some inspiration to take from thesignal
pattern?Describe alternatives you've considered
Open to discussion!
Additional context
See #68
The text was updated successfully, but these errors were encountered: