-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add route controller for better composability. (#565) #567
Add route controller for better composability. (#565) #567
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.
Looks like an excellent addition, thank you! One minor nit from my end - and a review by @adam-fowler who's on vacation right now.
/// Generates a middleware stack from the elements inside the result builder. The input, | ||
/// context and output types passed through the middleware stack are fixed and cannot be changed. | ||
extension MiddlewareFixedTypeBuilder { | ||
public static func buildExpression<C0: RouteController>(_ c0: C0) -> C0.Body where C0.Body.Input == Input, C0.Body.Output == Output, C0.Body.Context == Context { |
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've written all the other MiddlewareFixedTypeBuilder functions using MiddlewareProtocol. It would be great if you could continue that.
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'm not sure I follow the ask here. Given that RouteController
itself doesn't conform to MiddlewareProtocol
I think I need to constrain the generic to RouteController
. I might be misunderstanding the ask though.
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 updated the RouteController
to have a body that is a MiddlewareProtocol
and I also changed it to use @MiddlewareFixedTypeBuilder
instead. Maybe that's what you were referring to?
6148c97
to
6092824
Compare
There's definitely some ergonomic improvements that could be made here. Creating a high order controller with generic content gets a bit wordy and there is some object inference that I'm struggling with. It's also not super intuitive. I wonder what improvements we can make to simplify the concept? I'll be away from my comp for a few days, so won't be able to contribute much beyond discussion, but feel free to amend to this commit. struct FooContainerController<Content: RouterMiddleware>: RouteController where Content.Context: RouterRequestContext {
public typealias Context = Content.Context
@RouteBuilder<Context> var content: () -> Content
var body: some RouterMiddleware<Context> {
content()
}
}
struct BarContainerController<Content: MiddlewareProtocol>: RouteController where Content.Context: RouterRequestContext {
public typealias Context = Content.Context
@MiddlewareFixedTypeBuilder<Content.Input, Content.Output, Content.Context> var content: () -> Content
var body: some MiddlewareProtocol<Content.Input, Content.Output, Content.Context> {
content()
}
} Here, the inference of the body isnt't working.
let router = RouterBuilder {
FooContainerController {
Get { _,_ in "" }
}
} |
e800a01
to
60869b6
Compare
Description
Adds
RouteController
to improve the composability of routes when usingHummingbirdRouter
Conforming to
RouteController
allows you to create composable routes in a moreSwiftUI
result builder style.