-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Allow Routers to inherit state, take 2 #1368
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
Conversation
@davidpdrsn Have you had a look? I might have some time today or in the coming days to come back to this. |
d855343
to
52e9c56
Compare
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 like the direction of this! I think its a good balance between usability and type safety.
I'm fine with merging this without support for |
52e9c56
to
007b648
Compare
So I basically re-did everything except |
I also think we can get rid of passing state via |
007b648
to
68d33d7
Compare
Rebased to fix the unrelated trybuild failure. |
Added |
7cf1d13
to
21558eb
Compare
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 it looks good! I'll write some docs in a follow-up PR.
Wanna update the changelog? Then we can merge this 🚀
EDIT: We already have #1313 so wont file a separate issue.
2432ca0
to
a525d49
Compare
Motivation
Closes #1326.
Solution
Make
Router<S, _>
no longer a type that necessarily holds a state instance of typeS
. Instead, we track (at runtime, to avoid making the generics too complex) whether a givenRouter
instance wants to inherit its state.Status
I implemented router merging which was much easier than splitting
nest
intonest
andnest_service
. The fallback handler (currently stubbed out) will also need some special handling, but I don't think it should be a showstopper.