Parent macros are not available when rendering individual blocks #21

Ra0k · 2023-10-21T11:55:53Z

It would be a super cool feature if, in case of block-rendering, parent level macro imports were available for the blocks. For now, we either cannot use macros within blocks, which is very restricting, or need to import the macros in the blocks as well, which is uncomfortable and unintuitive. The worst part of it, is that one can never now for sure, without testing, if all block-rendering would work even if the complete template renders.

Desired syntax:

{% from 'macro.j2' import func %}

{{ func(global_variable) }}

{% block block_one %}
  {{ func(variable_1) }}
{% endblock %}

{% block block_two %}
  {{ func(variable_2) }}
{% endblock %}

Current workaround:

{% from 'macro.j2' import func %}

{{ func(global_variable) }}

{% block block_one %}
  {% from 'macro.j2' import func %}
  {{ func(variable_1) }}
{% endblock %}

{% block block_two %}
  {% from 'macro.j2' import func %}
  {{ func(variable_2) }}
{% endblock %}

sponsfreixes · 2024-07-01T00:19:26Z

Fixed by #30 .

gconklin · 2024-07-04T16:16:05Z

Hi @sponsfreixes - after doing more work on my own application, I'm discovering that the initial solution for this may in fact break a lot of other things. At this point, I'm not entirely sure what the path forward is, but I'm still researching.

I was very excited to get this part working since I had an immediate use for it, but now I'm finding that both of these PRs I made probably need to be reverted. (#30 & #33)

While this works for the simple template cases, more complex templates cause grief because (afaict) the only way to get jinja2 to generate the macros is to call make_template() and that call attempts to render the entire template file. The issue with that is, not all variables are available during the single block rendering. So variables outside of the block context also need to exist in order to have the entire template processed. For simple variables {{ thing }} it's not really an issue. But more complex components will raise Undefined errors.

More concretely:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>{{ mydict["title"] }}</title>{# undefined #}
    {% for k, v in mydict.items() %}{# undefined #}
        {{ k }} = {{ v }}
    {% endfor %}
    {{ obj.property }}{# undefined #}
</head>
<body>
    <h1>This is a header</h1>
    {% block content %}
        <p>This is the content. Hello {{ name }}!</p>
        <p>This is the bottom paragraph. Lucky number: {{ lucky_number }}</p>
    {% endblock %}
</body>
</html>

calling render_block for content will now fail because make_module() will try to render the entire template to extract any macros. In doing so, it will fail on mydict["title"], mydict.items(), and obj.property, for example.

Prior to my PR, that would have rendered the content block just fine.

I've spent several hours trying to figure out a way around this new discovery and haven't gotten anywhere meaningful. I apologize for not finding this earlier and wasting everyone's time.

I believe getting macros to work will be very beneficial, but not at the expense of what should be basic/original functionality.

sponsfreixes · 2024-07-04T23:09:53Z

Well that's unfortunate. Than you very for all the work and time you have put into it! I'd say the plan is for me to revert these changes and publish a stable version to PyPI. After that, I will definitely extend the test suite to cover more complex cases like the ones you describe. And after that, with whatever time we need, we try to figure out the macros. Unfortunately , I won't have access to my development PC until Saturday, so we will have to wait until then.

gconklin · 2024-07-05T17:30:18Z

I definitely agree with reverting.

I've been playing some more and have come up with a thought, but requires a modification to the render_block* calls by adding 1 or 2 default parms.

render_block calls template.new_context() - and new_context takes 2 additional params of shared and locals - I propose that we make those available to the user by adding them to render_block() such as:

def render_block(
    environment: Environment,
    template_name: str,
    block_name: str,
    shared: bool = False,                              # added for completeness [1]
    locals: t.Optional[t.Mapping[str, t.Any]] = None,  # added for usefulness [1]
    *args: typing.Any,
    **kwargs: typing.Any,
) -> str:

same for render_block_async - possibly need to bubble up to places like jinja2_fragments/flask.py : render_block as well

and then later:
ctx = template.new_context(vars=dict(*args, **kwargs), shared=shared, locals=locals)

With that mechanism in place, the user is free to inject whatever they want into the locals, including macros. Which puts the onus on them to properly load the macros.

An example can be documented and also state that the template should only have macros in it and not other template content.

def get_jinja_macros(template_name):
    template = current_app.jinja_env.get_or_select_template(template_name) # flask specific
    template_module = template.module
    macros = {
        m: getattr(template_module, m)
        for m in dir(template_module)
        if isinstance(getattr(template_module, m), Macro)
    }
    return macros

...
    macros = get_jinja_macros("mymacros.jinja2")
    render_template("main.html.jinja2", "content_block", locals=macros, myvar="something")
...

[1] - it's possible if a user has existing code with variables named shared or locals, this will cause problems for them.

It introduced a bug that breaks some complex templates. The templates of our test suite are too simple to be impacted by the bug, so we missed it. See comments on Issue #30: #21 (comment)

sponsfreixes · 2024-07-07T01:27:09Z

A new version (1.5.0) has been pushed reverting the changes. I'll give some thought to your proposal, I'm OK with being a feature where the users need to know that there are some caveats associated with it.

gconklin · 2024-07-07T16:37:49Z

I'll work up a PR based off of 1.5.0 for what I've put together that is working for me and we can move implementation discussion there. I think at the least, those jinja2 variables (shared/locals) should be exposed to the user when rendering fragments.

gconklin · 2024-07-12T03:49:48Z

Welp, I have made a discovery that I should have realized earlier. This works (for flask):

macros = get_jinja_macros("macros.html.jinj2")
return render_block("template.html.jinja2", "content", **macros, var1="thing", var2=2, ...)

In this case, the loaded macros are passed as variables, and because vars and locals wind up in the same dict (parent), the macros are found.
https://github.com/pallets/jinja/blob/f8323cf4042ab058ac5b11743614c63308798541/src/jinja2/runtime.py#L108

sponsfreixes · 2024-07-15T17:23:53Z

This is a great finding! Something similar will probably be possible for other frameworks. What's get_jinja_macros exactly doing?

gconklin · 2024-07-15T17:38:12Z

get_jinja_macros() is in an above comment, but basically it's just loading a (macro-only) template (aka, no other templating in there) and then tells jinja to parse it and extract the macro "functions" and finally returns a dict(symbol->function) which is then expanded to keyword args when passed to render_block

def get_jinja_macros(template_name):
    template = current_app.jinja_env.get_or_select_template(template_name) # flask specific
    template_module = template.module # [1]
    macros = {
        m: getattr(template_module, m)
        for m in dir(template_module)
        if isinstance(getattr(template_module, m), Macro)
    }
    return macros

[1] for async usage, you have to call a different method (make_module_async()) and probably other async-y stuff

sponsfreixes added the enhancement New feature or request label Dec 16, 2023

sponsfreixes mentioned this issue Jan 22, 2024

imports are not considered when used in the fragment/block #24

Closed

gconklin mentioned this issue Jun 28, 2024

inject macros into context of block being rendered #30

Merged

sponsfreixes closed this as completed Jul 1, 2024

gconklin mentioned this issue Jul 3, 2024

accessing dictionary.items() is undefined #32

Closed

sponsfreixes pushed a commit that referenced this issue Jul 7, 2024

Revert macros support

f92644b

It introduced a bug that breaks some complex templates. The templates of our test suite are too simple to be impacted by the bug, so we missed it. See comments on Issue #30: #21 (comment)

sponsfreixes reopened this Jul 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parent macros are not available when rendering individual blocks #21

Parent macros are not available when rendering individual blocks #21

Ra0k commented Oct 21, 2023 •

edited

Loading

sponsfreixes commented Jul 1, 2024

gconklin commented Jul 4, 2024

sponsfreixes commented Jul 4, 2024 via email •

edited

Loading

gconklin commented Jul 5, 2024 •

edited

Loading

sponsfreixes commented Jul 7, 2024

gconklin commented Jul 7, 2024

gconklin commented Jul 12, 2024

sponsfreixes commented Jul 15, 2024

gconklin commented Jul 15, 2024 •

edited

Loading

Parent macros are not available when rendering individual blocks #21

Parent macros are not available when rendering individual blocks #21

Comments

Ra0k commented Oct 21, 2023 • edited Loading

sponsfreixes commented Jul 1, 2024

gconklin commented Jul 4, 2024

sponsfreixes commented Jul 4, 2024 via email • edited Loading

gconklin commented Jul 5, 2024 • edited Loading

sponsfreixes commented Jul 7, 2024

gconklin commented Jul 7, 2024

gconklin commented Jul 12, 2024

sponsfreixes commented Jul 15, 2024

gconklin commented Jul 15, 2024 • edited Loading

Ra0k commented Oct 21, 2023 •

edited

Loading

sponsfreixes commented Jul 4, 2024 via email •

edited

Loading

gconklin commented Jul 5, 2024 •

edited

Loading

gconklin commented Jul 15, 2024 •

edited

Loading