-
Notifications
You must be signed in to change notification settings - Fork 45
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
fpdart v2.0.0 #147
base: main
Are you sure you want to change the base?
fpdart v2.0.0 #147
Conversation
outdated for now with new `Effect`
New release: fpdart v2.0.0-dev.3:
|
packages/fpdart/lib/src/effect.dart
Outdated
return deferred.wait<E>().__unsafeRun(context).then( | ||
(exit) => signal | ||
.failCause<E, L>(const Interrupted()) | ||
.__unsafeRun(context.withoutSignal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is a bug, when the parent signal is interrupted it isn't propagated to the child signal.
packages/fpdart/lib/src/effect.dart
Outdated
Effect<Null, L, R> provide(Context<E> context) { | ||
final env = context.env; | ||
final effect = env is ScopeMixin && !env.scopeClosable | ||
? alwaysIgnore(env.closeScope()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this could be an issue, as if there was a defect when try to close the scope, it will be swallowed / unhandled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some code to handle defects when closing the scope, anything you think is missing?
fpdart/packages/fpdart/lib/src/effect.dart
Lines 289 to 305 in f691c1e
Effect<E, L, R> _provideEnvCloseScope(E env) => | |
env is ScopeMixin && !env.scopeClosable | |
? Effect<E, L, R>.from( | |
(context) => _unsafeRun(context).then( | |
(exit) => switch (exit) { | |
Left(value: final value) => Left(value), | |
Right(value: final value) => | |
env.closeScope<E, L>()._unsafeRun(context).then( | |
(exit) => switch (exit) { | |
Left(value: final value) => Left(value), | |
Right() => Right(value), | |
}, | |
), | |
}, | |
), | |
) | |
: this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to close the scope regardless of success or failure.
packages/fpdart/lib/src/effect.dart
Outdated
Effect<Scope<E>, L, R> acquireRelease( | ||
Effect<Null, Never, Unit> Function(R r) release, | ||
) => | ||
withScope.tapEnv( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically you should make the resource acquisition uninterruptible here, would require some thought about how to make interruption toggle-able / regional.
Hey Sandro. Looking good, I've been playing around. Given
, how would you do a fold across the iterable result of a ProduceEffect using a HandleEffect to result in an Effect of type ApplyEffect? Feel free to suggest changes in the shapes of those definitions if you have a better starting approach. Thanks |
Also, curious if you have any general suggestions in naming type alias of an effect. Those names aren't the names I'm using verbatim, but I have been applying a naming convention where I prefix an alias with a verb matching the general action within the Effect that maps an Env value to the result and/or error it could produce. So for example, in the actual code I'm working on, I've implemented typedefs as such: typedef DispatchEffect<State, DispatchError, Event>
= Effect<State, DispatchError, Iterable<Event>>;
typedef StateEventHandlerEffect<State, HandlerError, Event>
= Effect<(State,Event), HandlerError, State>; Again, not 100% sure I'm shaping things here, maybe with this you'll be able to let me know if I'm applying the general use correctly. Thanks |
@SandroMaglione what editor do you use? In intelliJ, when I'm creating an Effect using Wondering if you might consider this alternative definition for EffectGen:
|
packages/fpdart/lib/src/effect.dart
Outdated
static Effect<E, L, void> sleep<E, L>(Duration duration) => Effect.from( | ||
(_) => Future.delayed( | ||
duration, | ||
() => const Right(null), | ||
), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to use Timer
+Completer
.
@tim-smart How would interruption work for sleep
? How can sleep
detect an interrupt and block timer?
static Effect<E, L, void> sleep<E, L>(Duration duration) => Effect.from( | |
(_) => Future.delayed( | |
duration, | |
() => const Right(null), | |
), | |
); | |
static Effect<E, L, void> sleep<E, L>(Duration duration) => Effect.from( | |
(_) { | |
final completer = Completer<Exit<L, void>>(); | |
Timer(duration, () { | |
completer.complete(const Right(null)); | |
}); | |
return completer.future; | |
}, | |
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First you will want to add a Effect.async/asyncInterrupt for callback apis, then use Effect.asyncInterrupt with the Timer api. Then hook up timer.cancel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tim-smart I added async
, but I am having some issues understanding the mental model for Deferred
and interruption.
Could you give me an example of how to manually interrupt an Effect
with code? Does this required Deferred
or is it about Context
? Are fibers needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sleep should be:
static Effect<E, L, void> sleep<E, L>(Duration duration) =>
Effect.asyncInterrupt(
(resume) {
final timer = Timer(duration, () {
resume.succeed(null);
});
return Effect.succeedLazy(() {
timer.cancel();
});
},
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for interrupting an Effect, here is some pseudo code:
final cancel = Effect.never.runCallback();
// some time later...
cancel();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://effect-ts.github.io/effect/effect/Effect.ts.html#runcallback
In dart maybe something like:
void Function() runCallback({ void Function(Exit<E, A> exit)? onExit })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is runCallback
the only way to manually trigger an interruption?
How would you test interruption with sleep
(or also asyncInterrupt
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Effect (typescript) you can pass an abort signal to Effect.runPromise, use the cancel callback from Effect.runCallback or call interrupt on fiber returned from Effect.runFork.
dart doesn't have a built in "cancellation" primitive, so you could use a Future potentially.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think is it possible to use Deferred
(or Context
) in fpdart to give access to interruption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too sure what you mean, might just have to try it out!
@ryanhanks-bestow could you provide a screenshot of IntelliJ that shows the type not inferred? Do you know of any limitation of IntelliJ regarding Records? |
Hello! Is there anything I could do to help make some progress on this? I'd really like to switch some code to FPDart, but I don't want to if it'll soon need to be rewritten. In addition, I really like Effect-TS and would love to see a Dart equivalent. |
The initial implementation is there. The main issue now is with composition. The dart language doesn’t support union types and type inference is limited. This means that you are required to specific manually all types, which is not ideal from a developer UX perspective. So much so that it doesn’t yet justify using fpdart's This may be solved by static metaprogramming, so I am actually waiting for some progress on the language. |
Ok then. Thank you for the explanation, it's very helpful for understanding the current progress. |
I don't expect the That being said, the API as it is now works and hopefully won't need to change too much. |
It's primarily for a personal project, so there's not really a "production".
Ok, thanks! I guess I'll give it a go and report back with any feedback. Thanks again for fpdart! |
Finally got some time to work on migrating some code to fpdart. Feedback:
P.S.: I'll update this comment if I have anything else to add instead of notifying everyone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments, feel free to disregard.
|
||
typedef Exit<L, R> = Either<Cause<L>, R>; | ||
|
||
sealed class Cause<L> implements Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to use Error
over Exception
? Error
s shouldn't be caught, but Failure
s should.
@@ -0,0 +1,12 @@ | |||
part of "effect.dart"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to use part
s? Circular dependencies?
Just curious.
@@ -0,0 +1,61 @@ | |||
import 'ordering.dart'; | |||
|
|||
class Order<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! I love Order
from Effect, looks similar.
Foldable2<_EitherHKT, L, R>, | ||
Alt2<_EitherHKT, L, R>, | ||
Extend2<_EitherHKT, L, R> { | ||
sealed class Either<L, R> extends IEffect<Never, L, R> { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
final class _OptionThrow { | ||
const _OptionThrow(); | ||
} | ||
part of "effect.dart"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a micro module be possible, like Effect's, but even lighter?
In my head, you could keep IEffect
as base, but keep Option
, Either
, etc from directly interacting with Effect
, only IEffect
. That would make migrating from v1 and integrating it into existing projects easier, by not requiring users to use the Effect system (it's obviously recommended to use the Effect portion), and comes with the side-effect of a (slightly) smaller learning curve, as you can use it in chunks.
I thought there were some APIs here that called Effect
. Doesn't look like anything really, so I think it should be documented that you can still use Option
, Either
, etc, without needing to adopt the effect system.
|
||
typedef Exit<L, R> = Either<Cause<L>, R>; | ||
|
||
sealed class Cause<L> implements Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, also, could this get its own module? I get that it's an internal detail, but Exit
feels different than Cause
. Maybe rename it rather than splitting it given the size of Exit
?
); | ||
|
||
/// {@category folding} | ||
Effect<E, Never, C> match<C>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has a semi-non-trivial definition, so I'm hesitant to suggest this, but maybe it'd be possible to use native pattern matching? i.e.
final val = switch ($.syncEither(effect)) {
Left() => ...,
Right() => ...,
}
rather than
final val = effect.match(
() => ...,
() => ...,
)
Yes, it's more verbose, but the native pattern matching feels much more "right" to me. The function feels like it's working around an issue from JS that isn't in Dart.
Been writing some more code with fpdart, and I definitely think this could be simpler. There're plenty of functions that could just be pattern-matching, which reduces complexity/learning curve, and is more flexible.
Hello! My Second try to use Effect. test code abstract interface class FileLoader { final class JsonFileLoader implements FileLoader {
} call Effect get exception Exception has occurred. I expect Either.Left If I right undertund examples. |
I'm unsure as well, the code should catch it, but if you look at the v2 examples, you'll see that they call Disclaimer: #155 discouraged me enough to wait for macros, so I haven't written much with fpdart in a few months. |
This PR is an open discussion on what's the best strategy to release fpdart v2 to minimize friction for users of the library
👇 Please comment below with any idea, comment, critique or opinion 👇
Problems with v1
Too many classes (
IO
,IOOption
,IOEither
,Task
,TaskOption
,TaskEither
,Reader
,State
,ReaderTaskEither
):from*
,to*
, e.g.toTask
,toTaskEither
)Do
constructor for each class, making the do notation harder to useToo much jargon: methods and classes are using terms from pure functional programming (math), less clear and hard to understand (e.g.
pure
,Reader
,chainFirst
,traverse
,Endo
).Typeclasses do not work well with dart and they cause a lot of overhead to maintain and understand. In fact, they are not necessary to implement the core of fpdart (they can be removed 💁🏼♂️).
Too many "utility functions" that may be considered outside of the scope of fpdart (e.g.
predicate_extension.dart
).fpdart v2 solution:
Effect
A single
Effect
class that contains the API of all other classes in v1 (similar toReaderTaskEither
).All Effect-classes derive from the same interface
IEffect
:Benefits
effect.dart
)factory Effect.gen
): the do notation also includesOption
andEither
(since both extendIEffect
)succeed
instead ofpure
)Effect
and how it works)Downsides