-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Introduce Router.In
, Router.Out
, Router.Shared
and Router
traits
#26
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks, please see questions/comments below.
/** The covariant side of the router. */ | ||
trait Out[+BasePage] extends Shared { | ||
/** @see [[Router.currentPageSignal]] */ | ||
def currentPageSignal: StrictSignal[BasePage] |
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.
Latest version LGTM except that def currentPageSignal
and def origin
both should be val-s. The origin is not supposed to be lazy or changeable, and currentPageSignal
can't really be implemented as a def
because it pretty much has to have side effects inside (so if we did implement it as a def, it would likely be incorrect) (also there should be no reason to implement it as a def).
Anything else to do here @raquo ? |
All is good, just need to find the time to merge & release |
Discord discussion: https://discordapp.com/channels/1020225759610163220/1020226231834255390/1340436623640367115