Idea: simplify api

[Alken0]

Hey,
I like what you did but would like to add suggestion (I actually wanted to create a similar crate in the first place).

It should be possible to extract the MatchedPath and Engine in the FromRequestParts Trait by introducing a Trait to extract the engine from the state (ideally the trait can be applied like FromRef via a macro).

This allows to render the tenplate via a simpler simpler function call and also it requires only 1 parameter instead of 2 for each handler.

I haven't tested if it works but it shoud simplify the api drastically for users.

[Alken0]

I am thinking of something like this as a minimal api:

async fn get_name(
    engine: TemplateEngine,
    Path(name): Path<String>,
) -> impl IntoResponse {
    let person = Person { name };

    engine.render(person)
}

// Define your application shared state
#[derive(Clone, FromRef, TemplateEngine)]
struct AppState {
    engine: TemplateEngine,
}

[Altair-Bueno]

Hi @Alken0, thanks for your feedback!

Key and TemplateEngine doesn't mix because it would either require runtime checks or enforce the use of Key, which is a deliberate design decision. Some examples use it, others use custom keys and lastly you may want to call the same template no matter what the current route path is. Render accepts anything that can be coerced to &str.

If your application doesn't benefit from these design decisions, you can always do any of the following and enforce it yourself across your application:

• Extend the TemplateEngine trait for method like invocations

pub trait TemplateEngineExt {
  // TODO
}

impl <EXT: TemplateEngine> TemplateEngineExt for EXT {
  // TODO
}

• Create your own FromRequestParts extractor, based on the Key and TemplateEngine implementations. See axum docs.

[Alken0]

I mean it would still be possible to implement a "CustomKey" more or less the same way it's done right now.

Also to support fixed-template-paths it's possible (and very senseful as it is a completly different usecase) to provide a different method with a path-parameter.

I just want to point out that the current approach requires lot's of repetitive and unnecessary code that could be reduced in every single handler.

[Altair-Bueno]

Just to make things clear, the API change you are proposing looks something like this:

// Using MatchedPath (Key). It renders the template whose name is the current path
async fn foo(engine: Engine<Handlebars<'static>>) -> impl IntoResponse {
	engine.render(())
}

// Using a custom key. It will render the template whose name is whatever CustomKey generates
async fn baz(engine: Engine<Handlebars<'static>, CustomKey>) -> impl IntoResponse {
	engine.render(())
}

// Using dynamic/static key. It will render the template named "bar"
async fn bar(engine: Engine<Handlebars<'static>>) -> impl IntoResponse {
    let name = "bar";
	engine.with_key(name).render(())
}

As I stated earlier, this would add runtime overhead to the third case (bar). The key would be extracted from the request even if it won't be used. So this can't be added to Engine.

The alternative I would be to define another extractor that leverages both the Key and the engine (second suggestion). Something like this:

pub struct Renderer<E, K = Key> {
	engine: E,
	key: K
}
// impl FromRequestParts for Renderer
impl<E,K> Renderer<E,K> {
	pub fn render<D>(self, data: D) -> Render<E,K,D> {
		Render(self.key, self.engine, data)
	}
    // TODO add render_html method
}

// Both Key and CustomKey can leverage this new Renderer extractor
async fn foo(renderer: Renderer<Engine<Handlebars<'static>>>) -> impl IntoResponse {
	renderer.render(())
}
async fn baz(renderer: Renderer<Engine<Handlebars<'static>>, CustomKey>) -> impl IntoResponse {
	renderer.render(())
}

// For dynamic/static keys, the raw engine is used to avoid the overhead
async fn bar(engine: Engine<Handlebars<'static>) -> impl IntoResponse {
    let name = "bar";
	Render(name, engine, ())
}

I'm not sure of the usefulness of this approach as it increases API complexity. In my experience, less is more. The moment you add more ways of doing the same thing, people start to get confused. On the other hand, I agree that this would reduce boilerplate for users who mostly render templates based on keys.

What do you think? Would Renderer suffix your needs?

[Alken0]

Yes indeed this would create an overhead. I am sorry, I overlooked it in the last comment.
And sorry for not formatting it correctly, yours looks so pretty :D.

Your suggestion is exactly what I meant.

What do you think about tweaking the code a little bit. This does not change any functionality but may be less confusing (even though a better wording than renderer.render is still needed):

// this was called Engine before
pub struct StaticRenderer<E> {
	engine: E //e.g. Handlebars
}
// impl FromRequestParts for Renderer
impl<E> StaticRenderer<E> {
	pub fn render<D>(self, key: String, data: D) -> Render<E,D> {
		Render(key, self.engine, data)
	}
}


// this was called Renderer before
pub struct DynamicRenderer<E, K = Key> {
	renderer: StaticRender,
	key: K
}
// impl FromRequestParts for DynamicRenderer
impl<E, K> DynamicRenderer<E, K> {
	pub fn render<D>(self, data: D) -> Render<E,K,D> {
		self.renderer.render(self.key, data)
	}
}

// Both Key and CustomKey can leverage this new DynamicRenderer extractor

type DynRender = DynamicRenderer<StaticRenderer<Handlebars<'static>>>
async fn foo(renderer: DynRender) -> impl IntoResponse {
	renderer.render(())
}
type DynRenderMyKey = DynamicRenderer<StaticRenderer<Handlebars<'static>>, MyKey>
async fn baz(renderer: DynRenderMyKey) -> impl IntoResponse {
	renderer.render(())
}

// For dynamic/static keys, the StaticRenderer is used to avoid the overhead
// For a user they seem very similar because of the
type StatRender = StaticRenderer<Handlebars<'static>
async fn bar(renderer: StatRender) -> impl IntoResponse {
	let template_path = "bar";
	renderer.render(template_path, ())
}

[Alken0]

And I don't want to push you to change your library.
I just want to suggest, feel free to say no at any time.

[Altair-Bueno]

Changing Engine to StaticRenderer is a no, at least for now, as It is a breaking change.

I've slept on this and I'm still not convinced there are enough benefits to do this. However, I would like to leave this issue open and allow anyone to propose new ideas.