-
Notifications
You must be signed in to change notification settings - Fork 321
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
Make Endpoint::call generic over lifetime #397
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 great!
} | ||
|
||
pub(crate) fn add_all(&mut self, path: &str, ep: impl Endpoint<State>) { | ||
self.all_method_router | ||
.add(path, Box::new(move |cx| Box::pin(ep.call(cx)))) |
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.
Oh, getting rid of this is nice!
impl<State> Clone for Service<State> { | ||
fn clone(&self) -> Self { | ||
Self { | ||
router: self.router.clone(), | ||
state: self.state.clone(), | ||
middleware: self.middleware.clone(), | ||
} | ||
} | ||
} | ||
|
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.
What's the difference with the Derive
block? It looks like there should be no difference?
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.
derive
block adds State: Clone
bound, which is undesirable.
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.
Oh I thought that bound was intentional. How else do we guarantee that the State can always be cloned for each route?
I feel like I'm missing something here 😅
@@ -21,7 +17,7 @@ use crate::{Endpoint, Request, Response}; | |||
/// app.listen("127.0.0.1:8080").await?; | |||
/// # | |||
/// # Ok(()) }) } | |||
/// ```` |
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.
ha, nice catch
This is a breaking change.
Make
Endpoint::call
generic over lifetime likeMiddleware::handle
.