twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.06k stars 1.24k forks source link

Twig Extension no longer rendering output #4110

Closed tacman closed 18 hours ago

tacman commented 4 days ago

Did something change in user-defined twig extensions? This is in the category of "I swear this used to work!" My extension is at https://github.com/tacman/twig-tree-tag.

It's a slick way of making recursive calls to tree elements, like menus

    {% set food = [
        {   name: 'fruits', 
            children: [{name: 'apple'},{name:'banana'}]        },
        {   name: 'veggies',
            children: [
            {name: 'peas'},
            {name: 'carrots'}
        ]
        }
    ] %}
    <h1>Food Tree</h1>
    {% tree item in food %}
        {% if treeloop.first %}<ul>{% endif %}
        <li>
            {{ item.name }}
            {% subtree item.children|default([]) %}
        </li>
        {% if treeloop.last %}</ul>{% endif %}
    {% endtree %}

{% endblock %}

It used to display the nested HTML, and now nothing. It's registered okay, but no content either. I've created a repo showing the bug

git clone git@github.com:tacman/twig-tree-tag-demo && cd twig-tree-tag-demo
composer install
symfony server:start -d
symfony open:local

It's a trival app -- install the library, register the extesion in services.yaml, and create a controller / template.

Did something change in twig regarding output? I vaguely remember something about twig and using the output buffer in a discussion.

It's a cool tag, it really simplifies rendering trees by skipping creating a macro to make recursive calls. I didn't write it, but I did upgrade it a few years ago to Twig 3, and I'm certain at some point the twig 3 version worked.

Thanks.

stof commented 3 days ago

Twig has a new yield-based implementation replacing the implementation based on output buffering (which will make it compatible with Fiber-based code as it avoids the global output buffering that would be shared between fibers). Twig has a BC layer for non-yield-ready nodes. But maybe this BC layer fails to automatically handle your TreeNode logic.

tacman commented 3 days ago

Thanks, @stof . Is there documentation for that yet? Or a relatively simple example I can follow? I've read https://twig.symfony.com/doc/3.x/advanced.html#defining-a-node but I can't see what I'm doing differently.

Or is it something that can be fixed in the BC layer?

fabpot commented 1 day ago

Here is the patch you need to apply to make it work:

diff --git a/src/Twig/Node/TreeNode.php b/src/Twig/Node/TreeNode.php
index 4f5fb0d..c637bdd 100644
--- a/src/Twig/Node/TreeNode.php
+++ b/src/Twig/Node/TreeNode.php
@@ -131,7 +131,7 @@ class TreeNode extends Node
         $compiler
             ->outdent()
             ->write("};\n")
-            ->write("\$tree_")
+            ->write("yield from \$tree_")
             ->raw($this->getAttribute('as'))
             ->raw("(")
             ->subcompile($this->getNode('seq'))

You will also need to remove the usage of the twig_ensure_traversable function as it's an internal function that has been removed.

The new yield implementation BC layer has many drawbacks that were not anticipated. I need to come back to the drawing board to see how we can mitigate these problems.

tacman commented 1 day ago

Thanks, it's working now!

I wasn't sure how to remove the call to twig_ensure_traversable, so I replaced it with the internal call that's used elsewhere:

            ->write("\$context['_seq'] = CoreExtension::ensureTraversable(\$data);\n")

Is there a better way?

fabpot commented 19 hours ago
ensureTraversable

That's the way to go indeed.

tacman commented 18 hours ago

Seems to be working fine now, thanks!

ericmorand commented 17 hours ago

@fabpot

That's the way to go indeed.

CoreExtension::ensureTraversable is internal too. How can this be the way to go if using twig_ensure_traversable, also internal, was not the way to go and was removed without notice - I quote you:

You will also need to remove the usage of the twig_ensure_traversable function as it's an internal function that has been removed.

fabpot commented 7 hours ago

@fabpot

That's the way to go indeed.

CoreExtension::ensureTraversable is internal too. How can this be the way to go if using twig_ensure_traversable, also internal, was not the way to go and was removed without notice - I quote you:

You will also need to remove the usage of the twig_ensure_traversable function as it's an internal function that has been removed.

"The way to go" just means that CoreExtension::ensureTraversable is the replacement for twig_ensure_traversable.But to make it clear, this is still internal and not recommended for usage in user space.

ericmorand commented 2 hours ago

@fabpot fair enough. Is there a recommended way?

fabpot commented 2 hours ago

@fabpot fair enough. Is there a recommended way?

That's really a one liner: return is_iterable($seq) ? $seq : [];