There are 2 issues in our Twig templating which can be fixed in a single change:
Attribute PHP objects are great when coding with PHP, but they don't belong to a Twig template:
create_attribute(mapping), which is now safe to use thanks to
📌
Prevent TypeError when using create_attribute Twig function
Fixed
, looks alien. A Twig function is supposed to create data, not transform data.{% set wrapper_attributes = create_attribute() %}
{% if wrapper_attributes %}<div{{ wrapper_attributes }}>{% endif %}Also, it would be great to help SDC component authors 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
{{ {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.
Twig template is supposed to be a "safe" job, so we must avoid any situation raising errors.
On Symfony 7, the same Twig snippet raises Warning: Array to string conversion.
So {{ {foo: "bar"} }} will be printed foo="bar"
Not sure where, not sure how.
For example, and without any tests, we can leverage Element::isRenderArray and 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.
We are not raising the error anymore.
"Chainable" methods from Attribute PHP object needs to be converted to filters:
add_class filter, available since last summer:
✨
Create twig filters: |add_class and |set_attribute
Fixed
set_attribute filter, available since last summer:
✨
Create twig filters: |add_class and |set_attribute
Fixed
If we have same add_class and set_attribute filters for renderable arrays and attributes mapping, we need to decide how to handle empty mappings:
What do we do with the other methods from Attribute PHP object?
is x in my_attributes|keys()?x in my_attributes["class"]?is x in my_attributes|keys()?my_attributes[x]?set_attribute?::removeAttribute?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 with minimal use of PHP objects.
Both ThemeManager et SDC are injecting a default attribute variable which is such an AttributePHP object.
This automatic injection is useful and must be kept. Maybe this variable will be a mapping instead of a PHP object in the future, but it will be a compatibility break.
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.
Let's try to not break anything yet.
Active
11.0 🔥
Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, 10.2 is in security mode now.
While this technically must be fixed in the theme system, I think this will only ever get attention in the SDC component, because that is the only place where people would experience the difference in practice.