Problem/Motivation
There is 2 issues in our Twig templating which can be fixed in a single change:
1. We use Attribute objects in templates.
Attribute PHP objects are great when coding with PHP, but they don't belong in a Twig template:
Also, it would be great to help SDC component author to stop using a PHP namespace as a prop type, which is not JSON Schema valid:
props:
type: object
properties:
wrapper_attributes:
title: Wrapper attributes
type: Drupal\Core\Template\Attribute
2. We have a fatal error when we print a mapping
{{ {foo: "bar"} }}
Raises:
InvalidArgumentException: "pop" is an invalid render array key. Value should be an array but got a string. in Drupal\Core\Render\Element::children() (line 97 of core/lib/Drupal/Core/Render/Element.php).
Because print nodes are caught by TwigNodeVisitor::leaveNode():
if ($node instanceof PrintNode) {
...
$class = get_class($node);
$line = $node->getTemplateLine();
return new $class(
new FunctionExpression('render_var', new Node([$node->getNode('expr')]), $line),
$line
);
}
Then TwigExtension::renderVar() calls the rendering service, considering any array is a render array.
On Symfony 7, the same Twig snippet raises Warning: Array to string conversion.
Proposed resolution
Print mapping as attributes
So {{ {foo: "bar"} }} will be printed foo="bar"
Not sure where, not sure how.
For example, and without any tests, we can leverage Element::isRenderArrayand act as soon as renderVar:
public function renderVar($arg) {
...
if (is_array($arg) && !Element::isRenderArray($arg)) {
$arg = new Attribute($arg);
}
if (is_object($arg)) {
...
}
...
return $this->renderer->render($arg);
}
Or later, in the renderer service.
Use filters to manipulate mappings
Since last summer, we have
✨
Create twig filters: |add_class and |set_attribute
Fixed
.
We can adapt them by testing if they are not render arrays and adding the class or the attribute directly in the mapping.
For add_class, it is mandatory, because there is a de-duplication logic.
However, set_attribute feature can also be done with usual mapping operations: my_attributes["role"] = "parent"
Which other methods from Attribute PHP object do we need to convert to filters?
- Attribute::getClass ➤ my_attributes["class"]
- Attribute::getIterator ➤ Useless because we already have an associative array
- Attribute::hasAttribute ➤
is x in my_attributes|keys()?
- Attribute::hasClass ➤ Seems useful
- Attribute::jsonSerialize ➤ https://twig.symfony.com/doc/3.x/filters/json_encode.html
- Attribute::merge ➤ https://twig.symfony.com/doc/3.x/filters/merge.html ?
- Attribute::offsetExists ➤
is x in my_attributes|keys()?
- Attribute::offsetGet ➤
my_attributes[x]?
- Attribute::offsetSet ➤ Same as
set_attribute?
- Attribute::offsetUnset ➤ Same as Attribute::removeAttribute?
- Attribute::removeAttribute >> Would Drupal's without() filter be useful?
- Attribute::removeClass >> Seems useful
- Attribute::storage >> Useless because we already have an associative array
- Attribute::toArray >> Useless because we already have an associative array
Change management
Once merged in Core, let's promote the use of attributes as mappings instead of PHP objects in our documentation, especially for SDC templates which are expected to be more "dumb" (it is a compliment) and UI oriented, so are expected to avoid the use of any PHP objects.
Out of scope
Default attribute variable
Both ThemeManager et SDC are injecting a default attribute variable which is such an AttributePHP object.
This automatic injection is useful and need to be kept. Maybe this variable will be a mapping instead of a PHP object in the future, but it will be a compatibility break.
Printing logic
Let's use the logic currently implemented in Attribute::__toString():
{ foo: "bar", baz: ["lorem", "ipsum"] }
foo="bar" baz="lorem ipsum"
{ foo: "bar", baz: [{"lorem": "hello"}, "ipsum"] }
foo="bar" baz="Array ipsum"
{ foo: "bar", baz: {"lorem": {"ipsum": "hello"} } }
foo="bar" baz="Array"
If we are unhappy with it, let's create a distinct ticket.
API changes
Let's try to not break anything yet.
