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

Arrow Fx DSL #3443

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Arrow Fx DSL #3443

wants to merge 9 commits into from

Conversation

nomisRev
Copy link
Member

This PR proposes a new module for Arrow Fx Coroutines, which covers the existing behavior in Arrow Fx but in a generalised way using DSLs.

All the above non-DSL variants, can be implemented by their DSL variations. Simplifying the internals of Arrow Fx Coroutines. The idea behind this is future proofing, since my ambition for 3.0 is to split Raise and the typed error handling DSL from Arrow Core such that users can rely on only the DSL after 3.0. That should be do-able with only a single binary breakage in Arrow Core, so 3.0 should be source compatible, and 99% binary compatible once Context Parameters are released.

  • Write tests
  • Check all edge-cases with AwaitAllScope
  • Implement parZipOrAccumulate DSL
  • Finish RacingScope

Investigate if we need, or want, <E> variants when inside Raise<E>.

@nomisRev nomisRev requested review from serras, raulraja and kyay10 May 30, 2024 09:35
@serras
Copy link
Member

serras commented May 31, 2024

One DSL to rule them all and in the coroutines bind them?

@nomisRev
Copy link
Member Author

nomisRev commented Jun 1, 2024

"It all began with the forging of the Great" DSLs 🤣

@nomisRev nomisRev added the 2.x.x label Oct 2, 2024
@nomisRev
Copy link
Member Author

nomisRev commented Oct 2, 2024

These can also come after 2.0.0 @serras

} catch (e: Throwable) {
throw e
}
override fun autoClose(close: (Throwable?) -> Unit) =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I'd like onClose better to reflect onRelease from ResourceScope

*/
// TODO should this be implemented on ScopingScope
// Or should ScopingScope be used to implement a separate SagaScope??
public suspend fun <A> ScopingScope.saga(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this just ScopingScope.resource?

import kotlinx.coroutines.withContext
import kotlin.coroutines.cancellation.CancellationException

public interface ScopingScope : AutoCloseScope {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is identical to ResourceScope, no? ScopingScope doesn't feel like a better name IMO.


public interface ScopingScope : AutoCloseScope {
public override fun autoClose(close: (Throwable?) -> Unit): Unit
public fun closing(block: suspend (Throwable?) -> Unit): Unit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, calling this onRelease might make for a more seamless transition.

import kotlin.coroutines.cancellation.CancellationException

public interface ScopingScope : AutoCloseScope {
public override fun autoClose(close: (Throwable?) -> Unit): Unit
Copy link
Collaborator

@kyay10 kyay10 Nov 2, 2024

Choose a reason for hiding this comment

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

this can have a default impl of: closing { close(it) }

@kyay10
Copy link
Collaborator

kyay10 commented Nov 3, 2024

Hey @nomisRev! Turns out I've accidentally reinvented some of your ScopingScope in #3518.
Do you think it'd make sense if this PR, instead of inventing a new ScopingScope, simply moves the (cleaned up) ResourceScope to this new DSL module (while also api importing it in fx proper so that no source or binary breakage occurs). Similarly, we can deprecate install, releaseCase, onRelease etc and deprecate ExitCase since it's equivalent to Throwable?.

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

Successfully merging this pull request may close these issues.

3 participants