Skip to content
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

Add fromFutureCancelable and friends #3374

Merged

Conversation

armanbilge
Copy link
Member

@armanbilge armanbilge commented Jan 18, 2023

This is a follow-up to:

I was working on http4s-dom and encountered roughly this:

F.fromPromise(F.delay(Fetch.fetch(...))).onCancel(F.delay(abortController.abort()))

The problem is that fromPromise is implemented via async_ which will be uncancelable under the new semantics. Without a fromPromiseCancelable variant I'd have to re-implement the conversion logic or run it on another fiber, so here we are.

Along these lines I also added fromFutureCancelable. I did not add a variant for CompletableFuture because its API natively supports cancelation (go Java! :trollface:). As a plus side, these methods offer an escape hatch for anyone who is bitten by the new semantics.

The method names and signatures feel a bit unergonomic so definitely open to bikeshed. I didn't try too hard to share implementations since it would also be messy.

@armanbilge armanbilge added this to the v3.5.0 milestone Jan 18, 2023
@armanbilge armanbilge changed the title Add 'fromFutureCancelable` and friends Add fromFutureCancelable and friends Jan 18, 2023
Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nesting on this pretty much prevents anything with better ergonomics. I think this is probably the best we can do, unfortunately.

@djspiewak djspiewak merged commit 22961a3 into typelevel:series/3.x Feb 14, 2023
@djspiewak djspiewak mentioned this pull request Feb 25, 2023
@He-Pin
Copy link

He-Pin commented Mar 12, 2023

lol:) refs: #160 too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants