mitsuhiko / minijinja

MiniJinja is a powerful but minimal dependency template engine for Rust compatible with Jinja/Jinja2
https://docs.rs/minijinja/
Apache License 2.0
1.56k stars 86 forks source link

Context not passed into state of custom functions when called within macro #535

Closed mbund closed 1 month ago

mbund commented 2 months ago

Description

When adding a function with add_function that accepts a &minijinja::State, the context provided at the "global" scope (with the context! macro) doesn't get passed in when within a macro's {% call %} block.

Reproduction steps

A reproduction repo can be found here.

Consider the following custom function myfunc

fn jinja_myfunc(state: &minijinja::State) -> String {
    let global = state.lookup("my_global");

    match global {
        Some(_) => "global exists".to_owned(),
        None => "global DOES NOT exist".to_owned(),
    }
}

And template main

{% macro wrapper() %} <wrapper>{{ caller() }}</wrapper> {% endmacro %}

{{ myfunc() }}

{% call wrapper() %}
  {{ var_a }}
  {{ myfunc() }}
{% endcall %}

{% call wrapper() %}
  {% with lift_global_up=my_global %}{% endwith %}
  {{ var_a }}
  {{ myfunc() }}
{% endcall %}

Now, rendering main with context! { var_a => "v_a", my_global => "my_global" } currently produces the following output:

global exists

<wrapper>
  v_a
  global DOES NOT exist
</wrapper>

<wrapper>

  v_a
  global exists
</wrapper>

The function myfunc expects the variable my_global to be in the current global state. Normally it exists "passively" as shown by the first global exists at the top. However, when entering the macro scope wrapper, the my_global variable is forgotten about even though the function requires looking it (as shown by the global DOES NOT exist).

The third <wrapper> shows my current workaround, where I can "lift" the variables I know my function needs into the current macro scope so the function runs properly.

Additional helpful information:

What did you expect

Variables passed in when rendering a template (with the render function and context! macro), should always be visible in minijinja state. The state should not be forgotten about when entering into a macro call.

mitsuhiko commented 1 month ago

I'm not saying it's intentional but it kinda of is a pretty intentional consequence of how closures are implemented. The macro wrapper does not reference (and thus enclose) my_global and so it won't be captured. If it was an actual global (as in added with Environment.add_global) it would show up.

That is unfortunate because it also means that certain patterns that really want to reference config state won't work, but I'm not sure if there is a reasonable way to prevent this.

mbund commented 1 month ago

Interesting. I wonder if it would be possible to mix the true global state and the context of the current rendering template for &minijinja::State in functions.

Or maybe something even more magical like axum extractors so you can define arguments for the variables you expect to be in scope and it can know what it needs to inject at compile time, like:

fn jinja_myfunc(ExtractedState(state): ExtractedState<MyExpectedState>) // ...

#[derive(Deserialize)]
struct MyExpectedState {
    my_variable: String,
}

I'm not familiar with the internals of this crate, so I'm not sure how difficult these suggestions might be to implement.

mitsuhiko commented 1 month ago

@mbund can you share an more real-world-ish example of how you are running into this?

mbund commented 1 month ago

Sure, to be specific:

I am rendering HTML, and have a card macro which just a div with some default styles, and then has a {{ caller() }} in it. I also have a function t which translates strings. Per request, I inject a lang in the context which reflects the language of the current user, so lang can't be an environment global. The t function reads this lang from the state to decide which language to translate into. It ends up looking something like this

{% call card.root() %}
  <h2>{{ t("account-header") }}</h2>
  <div>{{ t("account-content") }}</h2>
{% endcall %}

I suppose I could also modify my t function to take in the lang directly, but it becomes much more verbose, especially given how often I call the t function.

{% call card.root() %}
  <h2>{{ t("account-header", lang=lang) }}</h2>
  <div>{{ t("account-content", lang=lang) }}</h2>
{% endcall %}
mitsuhiko commented 1 month ago

I see the challenge. Let me have a look at what can be done here. One workaround for now would be to use a thread local.

mitsuhiko commented 1 month ago

This will be quite a bit better in the next release.

mitsuhiko commented 1 month ago

I will close this as resolved for now.