Replies: 2 comments 2 replies
-
Nice analysis! 😁 Personally, I do not like promises that neither resolve nor reject. It can cause bugs if you write code like this: setLoading(true);
await cancelableOperation();
setLoading(false); Your loading spinner would get shown forever if the operation gets canceled. 😉 I think this example shows that, in some cases, the code that triggered the asynchronous operation also needs a way to be notified if the operation is canceled. There also seems to be a fundamental difference between native promises (which As another data point, in C#, if a |
Beta Was this translation helpful? Give feedback.
-
Definitely, it'd be a problem when |
Beta Was this translation helpful? Give feedback.
-
Long-standing async tasks libraries like fluture or fun-task don't run the reject callback on cancellation. If possible, I think it would be great to follow the same path for
real-cancellable-promise
.It may look counterintuitive at first, but when you work with these libraries, you realize that it makes a lot of sense. The idea is that it was the consumer who canceled the operation, so it's redundant that the consumer should also get a rejection. The whole argument by the author or
fluture
, here.I think that the pattern we need to identify cancellation vs "normal exception" signals that something is off. Or the gymnastics with
rejectFn
in delay.I am aware that the scenario for
real-cancellable-promise
is a bit different, as it works withPromise
-like objects (thenables), so code likeawait cancellablePromise
would hang forever when canceled, right? In any case, it seems to me that one should always writeawait capture(cancellablePromise)
anyway, and the wrapper could then capture the cancellation. I am not familiar with the internals of the library, so there may be extra problems for this to be implemented. Something to investigate: fluture-js/Fluture#216 (comment).In any case, thank you for the project and especially for
buildCancellablePromise
! I used the same pattern to write an equivalent wrapper forfluture
, and it works nicely.Beta Was this translation helpful? Give feedback.
All reactions