Add the render function, deprecate the include one

[fabpot]

No description provided.

[fabpot]

Maybe it would be good to also deprecate the include tag in 3.15.
My only "concern" is about the embed tag that is kind of a glorified include node behind the scene.

[alexander-schranz]

@fabpot wanted to give some feedback. I really like the new include decision without context by default. Even I'm not a fan of renaming it render, but I understand from BC reasons for better transparency. I'm working the last two years where we only use with_context = false to make things more transparent what variables are accessed by components. Only case which was confusing is the embed tag which we use for things like overlays, fieldsets, ...

Typically looks like this:

{# overlay.html.twig #}
<dialog>
   <h3>
       {% block title %}{% endblock %}
   </h3>
   
   <div>
       {% block content %}{% endblock %}
   </div>
</dialog>

{# using.html.twig #}
{% set title = 'Title' %}
{% set content = 'Content' %}

{% embed "overlay.html.twig" only %}
    {% block title %}
        {{ title }} {# not available #}
    {% endblock %}
    {% block content %}
        <div>
            {{ content }} {# not available #}
        </div>
    {% endblock %}
{% endembed %}

To get this work I need give the variables also to overlay.html.twig which may have similar variables defined.

{# using.html.twig #}
{% set title = 'Title' %}
{% set content = 'Content' %}

{% embed "overlay.html.twig" {title: title, content: content} only %}
    {% block title %}
        {{ title }}
    {% endblock %}
    {% block content %}
        <div>
            {{ content }}
        </div>
    {% endblock %}
{% endembed %}

So it is a little bit strange. That I need to give the variables to the overlay.html.twig to access them in my own twig file. Better would be only have to give variables in it which are really used by the overlay.html.twig and block of the embed still has access to the one by its own file:

{# using.html.twig #}
{% set title = 'Title' %}
{% set content = 'Content' %}

{% embed "overlay.html.twig" only %}
    {% block title %}
        {{ title }} {# still have access to this one #}
    {% endblock %}
    {% block content %}
        <div>
            {{ content }} {# still have access to this one #}
        </div>
    {% endblock %}
{% endembed %}

Another solution would be maybe make it transparency on the embed block e.g.:

{# using.html.twig #}
{% set title = 'Title' %}
{% set content = 'Content' %}

{% embed "overlay.html.twig" only %}
    {% embedblock title with { title: title} %}
        {{ title }}
    {% endembedblock %}

    {% embedblock content with { content: content} %}
        <div>
            {{ content }}
        </div>
    {% endembedblock %}
{% endembed %}

I hope I did make it understandable where the issues with the embed tag is.

[stof]

render will conflict with the Symfony function rendering the response of a sub-request, which exists since 10 years in the Symfony ecosystem ? https://github.com/symfony/symfony/blob/v7.1.6/src/Symfony/Bridge/Twig/Extension/HttpKernelExtension.php#L28

[stof]

@alexander-schranz the {% embed %} tag actually defines an anonymous template extending the provided template name, with the body of the tag being the body of that anonymous template. The blocks inside it are normal blocks, but they are blocks defined by the anonymous template.

Your proposed syntax will not work, because blocks are rendered with the context from the caller (i.e. the parent template). they have no way to access the context of the place including the template.

[alexander-schranz]

@stof I understand that it is a technical challange to change that behaviour. Just wanted to give feedback that from end user perspective it just felled for me strange how it behaves currently.

[willrowe]

@fabpot is the new terminology variables as opposed to context or does context have a different meaning?

[derrabus]

render will conflict with the Symfony function

Maybe we can keep the include function, but add a flag to opt into the new behavior and deprecate not doing so.

[stof]

@derrabus technically, that flag already exists in include: with_context = false

[derrabus]

@derrabus technically, that flag already exists in include: with_context = false

Perfect. So let's deprecate not setting that flag to false explicitly?

[willrowe]

I thought @derrabus meant a flag in the configuration, like how you can ignore missing variables.

[stof]

@willrowe setting a flag in the Environment is bad, as it expects that the migration is done all at once, including all third-party templates. Switching the behavior of a function through a global state basically makes it unreliable for third-party code.

[lyrixx]

IMHO, twig should ship something (rector plugin, raw binary script), to update ours templates

[lyrixx]

Maybe it would be good to also deprecate the include tag in 3.15.

Yes, Let's deprecate the include tag too

[alexander-schranz]

@lyrixx think twig-cs-fixer from @VincentLanglet can do that work. Upgrading include without_context = false to the new render I think should be possible without lot of effort.

Problem is more about include where the context was not false which was default at current state. Sure quickfix could be give everywhere _context in it with a fixer rule but think that is something I would not suggest todo, but is sure the "easiest" upgrade way.

Example: VincentLanglet/Twig-CS-Fixer#327

For such rule it is sure easier if include get renamed to render. As the upgrade is then easier. Else a "fixer" is not able to skip already converted includes only if we would make still possible to set with_context = false to mark already converted includes, even true throws an error.

[smnandre]

Passing a Template instead of a TemplateWrapper is deprecated, afaik

[smnandre]

Suggested change

	"include" function
	"render" function