You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have a few thoughts on the initial implementation:
I'm not sure about WebTemplate rather than using a web framework specific name e.g.IntoResponse / Responder / Reply. This might make sense from the perspective of writing this code (all web frameworks are a thing, so let's provide interaction with them generically), but it falls down when you take the perspective of a user of each web framework. It would be rare to have multiple frameworks in the same project (i.e. axum and actix / warp). Choosing a name that shows what the derive macro will produce seems like it comes closer to idiomatic rust.
For axum, the IntoResponse trait is defined in axum-core, which in theory should be more stable than the main axum crate. It might be worth making the feature flag use core versions rather than axum versions to avoid the having to update so often, and additionally using the exported trait from that crate rather than the main crate.
There might be another approach that handles errors in a more axum friendly way instead of hard coding this to return a 500. Axum handles situations where the handler returns Result<impl IntoResponse, impl IntoResponse> nicely. If the generated code still generated some method on the template which returns some type which can be inspected to check the template, as well as any rendering error which can be fed into an app specific error handler, then the need for tracing / log / eprintln features might go away.
Where TemplateResponse then implements IntoResponse infallibly, but also makes it possible to inspect the template.
Not 100% certain this makes sense - just an idea at this point, but this feels like it's more aligned with the error handling approach in axum (not sure how familiar you are with that). Also I suspect some of this seems like it might require more askama internals to implement, which perhaps puts this in the basket of not wanting to head that direction.
The text was updated successfully, but these errors were encountered:
Calling the derive macro like the trait would clash with existing derive macros like rocket::Responder, (and I wanted to keep the needed work to maintain the library low, only using one pattern that works well enough for all web frameworks). I guess the name WebTemplate is probably "good enough". IMHO it tells exactly what it does, and it prevents (unlikely) future clashes if other web frameworks decide to implement a derive macro for their {IntoResponse,Reply,Scribe} traits.
Actually I already implemented it this way: axum-0.8 → axum-core-0.5 → dep:axum-core. I made it more clear that you can also explicitly use the feature axum-core-0.5 in the readme.
I guess .render_axum()? works almost exactly the same as .render().map(Html)?. The approach is already explained in frameworks § axum, but yeah, it might be a good idea to give the user the hint that they could add a trait to add a render_axum() method. I think in e.g. rocket you would just return an anyhow::Error, that you downcast in an error handler to present a message, so the "right" approach would still be framework specific.
Hey there, thanks for creating this.
I have a few thoughts on the initial implementation:
WebTemplate
rather than using a web framework specific name e.g.IntoResponse
/Responder
/Reply
. This might make sense from the perspective of writing this code (all web frameworks are a thing, so let's provide interaction with them generically), but it falls down when you take the perspective of a user of each web framework. It would be rare to have multiple frameworks in the same project (i.e. axum and actix / warp). Choosing a name that shows what the derive macro will produce seems like it comes closer to idiomatic rust.IntoResponse
trait is defined in axum-core, which in theory should be more stable than the main axum crate. It might be worth making the feature flag use core versions rather than axum versions to avoid the having to update so often, and additionally using the exported trait from that crate rather than the main crate.Result<impl IntoResponse, impl IntoResponse>
nicely. If the generated code still generated some method on the template which returns some type which can be inspected to check the template, as well as any rendering error which can be fed into an app specific error handler, then the need for tracing / log / eprintln features might go away.E.g. something like:
Where TemplateResponse then implements
IntoResponse
infallibly, but also makes it possible to inspect the template.Not 100% certain this makes sense - just an idea at this point, but this feels like it's more aligned with the error handling approach in axum (not sure how familiar you are with that). Also I suspect some of this seems like it might require more askama internals to implement, which perhaps puts this in the basket of not wanting to head that direction.
The text was updated successfully, but these errors were encountered: