-
Notifications
You must be signed in to change notification settings - Fork 59
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
[core] introduce reactive Signal #1082
base: main
Are you sure you want to change the base?
Conversation
1ef568f
to
c1a337f
Compare
@@ -13,7 +13,7 @@ import kyo.Result | |||
/** This class provides a jvm-specific implementation of signal handling. It uses reflection to install signal handlers as they may not be | |||
* available on all implementations of the JVM. | |||
*/ | |||
private[internal] class SignalPlatformSpecific: | |||
private[internal] class OSSignalPlatformSpecific: |
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.
How about calling this SignalHandler
? OSSIgnal
is a bit hard to read.
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 think SignalHandler
could still be confusing with the new Signal
impl. Maybe it's ok to be more opaque since it's an internal API? Would OsSignal
work better?
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.
Os
might be better
* @tparam A | ||
* The type of value contained in the signal. Must have an instance of `CanEqual[A, A]` | ||
*/ | ||
abstract class Signal[A](using CanEqual[A, A]): |
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.
Should this be sealed
since we have initRaw
?
* @tparam A | ||
* The type of value contained in the reference. Must have an instance of `CanEqual[A, A]` | ||
*/ | ||
final class Ref[A] private[Signal] (_unsafe: Ref.Unsafe[A])(using CanEqual[A, A]) extends Signal[A]: |
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.
Should we avoid the Ref
class name if we plan to have a similar class for an AtomicRef
?
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.
Good point, I think it can be confusing especially given that other effect systems use Ref
for the equivalent of Kyo's AtomicRef
. FS2 uses SignallingRef
but sounds a bit heavyweight to me. Maybe SignalRef
exported to the kyo
package?
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.
SignalRef
is good with me.
This new effect is in honor of the 600th person to start the repository. Congrats and thank you @diegobernardes!! 🎉
Problem
Kyo doesn't offer a solution to handle reactive signals, which is a common need in UI solutions for example.
Solution
Introduce
Signal
tokyo-core
with mutable and constant versions and methods to stream the state.Notes
Signal
/SignallingRef
but the implementation is more optimized with two separateAtomicRef
s to manage the state and listeners, which reduces allocations.updateAndGet
+getAndUpdate
for consistency.