Skip to content

Conversation

@rahulrangers
Copy link
Contributor

Previously, calling .mapK on a SignallingRef[F, A] would yield a plain Ref[G, A], causing the extra signaling behavior (such as discrete, continuous) to be lost. This PR introduces a new helper class, TransformedSignallingRef, which implements mapK for SignallingRef so that all signal-specific operations are lifted to the new effect type. This change ensures that after a transformation, the resulting reference remains a full SignallingRef[G, A] with its intended functionality.

@rahulrangers
Copy link
Contributor Author

@armanbilge Could you please review this and let me know if there are any mistakes that need correction?

@armanbilge armanbilge self-requested a review March 11, 2025 17:57
@rahulrangers
Copy link
Contributor Author

I think we can convert waitUntil with the transformed discrete and can you give me any idea like how can i try to override getAndDiscreteUpdates @armanbilge

@rahulrangers rahulrangers requested a review from armanbilge March 12, 2025 12:13
@rahulrangers rahulrangers requested a review from armanbilge March 12, 2025 15:56
@rahulrangers
Copy link
Contributor Author

I'm encountering an issue while implementing the waitUntil method in the TransformedSignallingRef class. Specifically, the compiler raises an error indicating that it cannot find an implicit value for the parameter F: cats.effect.kernel.Concurrent[F].

@rahulrangers rahulrangers requested a review from armanbilge March 12, 2025 18:50
@rahulrangers rahulrangers requested a review from armanbilge March 13, 2025 18:47
@rahulrangers rahulrangers requested a review from armanbilge March 13, 2025 19:07
@armanbilge armanbilge linked an issue Mar 13, 2025 that may be closed by this pull request
Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

Great, thanks, this looks good!

One final thing, can we add a test? Mostly I'm interested in a test to check that SignallingRef#mapK returns a SignallingRef instead of a Signal ... because we have two methods with the same name, I'm not entirely sure how they get resolved.

@rahulrangers
Copy link
Contributor Author

I'm note entirely sure how they get resolved.

I think it is resolved as SignallingRef's mapK has an extra implicit parameter making it different from Signal’s mapK.

@rahulrangers rahulrangers requested a review from armanbilge March 13, 2025 20:38
Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

It does seem to be working!

I think it is resolved as SignallingRef's mapK has an extra implicit parameter making it different from Signal’s mapK.

That's right, they are different. I am just not sure how the compiler decides which method to choose when it's ambiguous.

rahulrangers and others added 2 commits March 14, 2025 13:19
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
@rahulrangers rahulrangers requested a review from armanbilge March 14, 2025 08:12
Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

nice work, thank you!

@armanbilge armanbilge changed the title Add TransformedSignallingRef to Enable Proper mapK Lifting for SignallingRef Implement {Signal,SignallingRef}#mapK Mar 15, 2025
@armanbilge armanbilge merged commit c6de493 into typelevel:main Mar 15, 2025
16 checks passed
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.

Add TransformedSignallingRef a la TransformedRef

2 participants