yield used when use_yield = false

[brandonkelly]

Just getting around to updating Craft CMS’s Twig requirement to 3.10 from 3.8.

Some Craft-provided Twig nodes are breaking because they rely on output buffering to capture subcompiled node output, for example:

https://github.com/craftcms/cms/blob/c0a690ae58c795de61edb725460f87d9ddeae292/src/web/twig/nodes/RegisterResourceNode.php#L39-L42

It looks like those could be fixed by utilizing CaptureNode instead of output buffering, however it will impact third party plugins as well, so we can’t release this as-is, at least until the next major version.

Shouldn’t PrintNode et al. be taking Environment::useYield() into account before yielding their output, rather than echoing it? So everything continues to work as before up until Twig is instantiated with use_yield = true.

[nicolas-grekas]

Can you please check if #4216 fixes the issue with nodes that use output buffering?

[brandonkelly]

This is still not quite working for nodes that call subcompile() and expect any raw output to be echoed out, and captured in an output buffer, e.g. like this:

https://github.com/craftcms/cms/blob/6183b53d6aadab0808e673bf1deed675d9f408e4/src/web/twig/nodes/TagNode.php#L27-L33

You’ll end up with yield statements rather than echo statements:

// line 87
ob_start();
// line 90
yield "    ";
…

[nicolas-grekas]

Please open a new issue and provide a reproducer if possible.

[brandonkelly]

Isn’t it the same issue? I specifically referenced PrintNode in the OP.

[nicolas-grekas]

OK, reopening. But I'm missing a realworld description: the fix involves echoing strings that where yielded, so that ob_start() should catch yielded strings. Which means that as is, the updated report is incomplete. Can you please share a reproducer?

[fabpot]

Reproducer:

<?php

require_once __DIR__.'/vendor/autoload.php';

use Twig\Compiler;
use Twig\Environment;
use Twig\Extension\AbstractExtension;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Node;
use Twig\Node\PrintNode;
use Twig\Token;
use Twig\TokenParser\AbstractTokenParser;

error_reporting(-1);

class EchoNode extends Node
{
    public function compile(Compiler $compiler): void
    {
        $compiler
            ->addDebugInfo($this)
            ->write("ob_start();\n")
            ->subcompile($this->getNode('content1'))
            ->write('echo "\n";', "\n")
            ->write("echo 'foo';\n")
            ->write('echo "\n";', "\n")
            ->subcompile($this->getNode('content2'))
            ->write('echo "\n";', "\n")
            ->write("echo ob_get_clean();\n")
        ;
    }
}

class EchoTokenParser extends AbstractTokenParser
{
    public function parse(Token $token): Node
    {
        $this->parser->getStream()->expect(Token::BLOCK_END_TYPE);

        return new EchoNode([
            'content1' => new PrintNode(new ConstantExpression('content1', -1), -1),
            'content2' => new PrintNode(new ConstantExpression('content2', -1), -1),
        ]);
    }

    public function getTag(): string
    {
        return 'echo';
    }
}

class EchoExtension extends AbstractExtension
{
    public function getTokenParsers(): array
    {
        return [
            new EchoTokenParser(),
        ];
    }
}

$twig = new Environment(new Twig\Loader\ArrayLoader(['_bug.twig' => "HERE\n{% echo %}\nHERE"]), [
    'strict_variables' => true,
    'debug' => true,
    'cache' => false,
    'autoescape' => false,
    'use_yield' => false,
]);
$twig->addExtension(new EchoExtension());

$template = '_bug.twig';
echo $twig->parse($twig->tokenize(new Twig\Source($twig->getLoader()->getSourceContext($template)->getCode(), $template)))."\n\n";
echo $twig->compile($twig->parse($twig->tokenize(new Twig\Source($twig->getLoader()->getSourceContext($template)->getCode(), $template))))."\n\n";

$template = $twig->load($template);
echo $template->render([]);

[brandonkelly]

@fabpot Still seeing an issue after #4221 when macros are involved.

Here’s an example:

{% macro button() %}
  {% tag 'button' %}
    Button Label
  {% endtag %}
{% endmacro %}
{{ _self.button() }}

use Twig\Token;
use Twig\TokenParser\AbstractTokenParser;

class TagTokenParser extends AbstractTokenParser
{
    public function getTag(): string
    {
        return 'tag';
    }

    public function parse(Token $token): TagNode
    {
        $lineno = $token->getLine();
        $expressionParser = $this->parser->getExpressionParser();
        $stream = $this->parser->getStream();

        $nodes = [
            'name' => $expressionParser->parseExpression(),
        ];

        $stream->expect(Token::BLOCK_END_TYPE);
        $nodes['content'] = $this->parser->subparse(function(Token $token) {
            return $token->test('endtag');
        }, true);
        $stream->expect(Token::BLOCK_END_TYPE);

        return new TagNode($nodes, [], $lineno, $this->getTag());
    }
}

use Twig\Compiler;
use Twig\Node\Node;

class TagNode extends Node
{
    public function compile(Compiler $compiler): void
    {
        $compiler
            ->addDebugInfo($this)
            ->write("ob_start();\n")
            ->subcompile($this->getNode('content'))
            ->write("\$content = ob_get_clean();\n")
            ->write("echo '<' . ")
            ->subcompile($this->getNode('name'))
            ->raw(" . '>';\n")
            ->write("echo \$content;\n")
            ->write("echo '</' . ")
            ->subcompile($this->getNode('name'))
            ->raw(" . '>';\n");
    }
}

On Twig 3.8.0 that results in:

protected function doDisplay(array $context, array $blocks = [])
{
    $macros = $this->macros;
    // line 6
    echo twig_call_macro($macros["_self"], "macro_button", [], 6, $context, $this->getSourceContext());
    echo "
";
}

// line 1
public function macro_button(...$__varargs__)
{
    $macros = $this->macros;
    $context = $this->env->mergeGlobals([
        "varargs" => $__varargs__,
    ]);

    $blocks = [];

    ob_start();
    try {
        // line 2
        echo "  ";
        ob_start();
        // line 3
        echo "    Button Label
";
        $content = ob_get_clean();
        echo '<' . "button" . '>';
        echo $content;
        echo '</' . "button" . '>';

        return ('' === $tmp = ob_get_contents()) ? '' : new Markup($tmp, $this->env->getCharset());
    } finally {
        ob_end_clean();
    }
}

  <button>    Button Label
  </button>

But on 7173182 that gives me:

protected function doDisplay(array $context, array $blocks = [])
{
    $macros = $this->macros;
    // line 6
    yield CoreExtension::callMacro($macros["_self"], "macro_button", [], 6, $context, $this->getSourceContext());
    yield "
";
    return; yield '';
}

// line 1
public function macro_button(...$__varargs__)
{
    $macros = $this->macros;
    $context = $this->env->mergeGlobals([
        "varargs" => $__varargs__,
    ]);

    $blocks = [];

    return ('' === $tmp = \Twig\Extension\CoreExtension::captureOutput((function () use (&$context, $macros, $blocks) {
        // line 2
        yield "  ";
        ob_start();
        // line 3
        yield "    Button Label
";
        $content = ob_get_clean();
        echo '<' . "button" . '>';
        echo $content;
        echo '</' . "button" . '>';
        return; yield '';
    })())) ? '' : new Markup($tmp, $this->env->getCharset());
}

      Button Label
  <button></button>

[nicolas-grekas]

Thanks for spotting this. #4242 fixes it.

[fabpot]

@brandonkelly Can you confirm that all is good now? If yes, I will release 3.12 soon.

[brandonkelly]

Working great now!

[emodric]

Hi @fabpot ,

I'm still seing this (or similar) behaviour with Twig 3.12.0.

My compiled template looks like this and the effect is that the output of displayZone, which uses echo, is printed before any other yield here, or really anything else.

So Twig still uses yield even if some nodes do not declare yield compatibility.

    protected function doDisplay(array $context, array $blocks = [])
    {
        $macros = $this->macros;
        $__internal_b91a4435ea3baf1e2b6bfda56133dace = $this->extensions["Sentry\\SentryBundle\\Tracing\\Twig\\TwigTracingExtension"];
        $__internal_b91a4435ea3baf1e2b6bfda56133dace->enter($__internal_b91a4435ea3baf1e2b6bfda56133dace_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "template", "@nglayouts_app/layout/layout_2.html.twig"));

        $__internal_5a27a8ba21ca79b61932376b2fa922d2 = $this->extensions["Symfony\\Bundle\\WebProfilerBundle\\Twig\\WebProfilerExtension"];
        $__internal_5a27a8ba21ca79b61932376b2fa922d2->enter($__internal_5a27a8ba21ca79b61932376b2fa922d2_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "template", "@nglayouts_app/layout/layout_2.html.twig"));

        $__internal_6f47bbe9983af81f1e7450e9a3e3768f = $this->extensions["Symfony\\Bridge\\Twig\\Extension\\ProfilerExtension"];
        $__internal_6f47bbe9983af81f1e7450e9a3e3768f->enter($__internal_6f47bbe9983af81f1e7450e9a3e3768f_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "template", "@nglayouts_app/layout/layout_2.html.twig"));

        yield from $this->getParent($context)->unwrap()->yield($context, array_merge($this->blocks, $blocks));
        
        $__internal_b91a4435ea3baf1e2b6bfda56133dace->leave($__internal_b91a4435ea3baf1e2b6bfda56133dace_prof);

        
        $__internal_5a27a8ba21ca79b61932376b2fa922d2->leave($__internal_5a27a8ba21ca79b61932376b2fa922d2_prof);

        
        $__internal_6f47bbe9983af81f1e7450e9a3e3768f->leave($__internal_6f47bbe9983af81f1e7450e9a3e3768f_prof);

    }

    // line 3
    public function block_layout($context, array $blocks = [])
    {
        $macros = $this->macros;
        $__internal_b91a4435ea3baf1e2b6bfda56133dace = $this->extensions["Sentry\\SentryBundle\\Tracing\\Twig\\TwigTracingExtension"];
        $__internal_b91a4435ea3baf1e2b6bfda56133dace->enter($__internal_b91a4435ea3baf1e2b6bfda56133dace_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "block", "layout"));

        $__internal_5a27a8ba21ca79b61932376b2fa922d2 = $this->extensions["Symfony\\Bundle\\WebProfilerBundle\\Twig\\WebProfilerExtension"];
        $__internal_5a27a8ba21ca79b61932376b2fa922d2->enter($__internal_5a27a8ba21ca79b61932376b2fa922d2_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "block", "layout"));

        $__internal_6f47bbe9983af81f1e7450e9a3e3768f = $this->extensions["Symfony\\Bridge\\Twig\\Extension\\ProfilerExtension"];
        $__internal_6f47bbe9983af81f1e7450e9a3e3768f->enter($__internal_6f47bbe9983af81f1e7450e9a3e3768f_prof = new \Twig\Profiler\Profile($this->getTemplateName(), "block", "layout"));

        // line 4
        yield "<div class=\"zone-layout-layout2\">

    <section class=\"zone zone-header\">
        ";
        // line 7
        $this->env->getRuntime("Netgen\Bundle\LayoutsBundle\Templating\Twig\Runtime\RenderingRuntime")->displayZone(...);
        // line 8
        yield "    </section>

</div>
";
        
        $__internal_6f47bbe9983af81f1e7450e9a3e3768f->leave($__internal_6f47bbe9983af81f1e7450e9a3e3768f_prof);

        
        $__internal_5a27a8ba21ca79b61932376b2fa922d2->leave($__internal_5a27a8ba21ca79b61932376b2fa922d2_prof);

        
        $__internal_b91a4435ea3baf1e2b6bfda56133dace->leave($__internal_b91a4435ea3baf1e2b6bfda56133dace_prof);

        return; yield '';
    }

[fabpot]

@emodric Can you create a new issue? Also, if you can share a template that I can use to reproduce, that would help tremendously. Thank you.

[emodric]

@fabpot After further examination, it turns out this is endemic to one of our projects, so clearly there's some (probably incompatible) code somewhere affecting the Twig output. Sorry for the noise!

[watershed]

I can’t profess to understand the issue here, but its fallout is going to be huge for me. Marion’s plugin has been fundamentally useful for me. I’ll have to rethink everything :(

[stof]

that plugin is describing itself as hacking Twig macros. This has never been something covered by any BC guarantee of Twig.

[watershed]

I know. I’m not asking for anything. Just highlighting that I’m among about 1,000 domains that have a world of refactoring to do :(

[brandonkelly]

@watershed Definitely not something Twig itself should have to worry about. I just PR’d a fix for it here: marionnewlevant/craft-twig_perversion#27

[watershed]

Agreed. And thank you Brandon

[vitaliiivanovspryker]

What relaese does it fix?

[xabbuh]

#4242 was first released with Twig 3.12.