
Problem/Motivation
If I set a single-directory component slot to an empty array, I will get this error:
Drupal\Core\Render\Component\Exception\InvalidComponentDataException: Unable to render component "MODULE:COMPONENT". A render array or a scalar is expected for the slot "SLOT" when using the render element with the "#slots" property in Drupal\Core\Render\Element\ComponentElement->generateComponentTemplate() (line 118 of /.../core/lib/Drupal/Core/Render/Element/ComponentElement.php).
Setting empty values to NULL also does not work. If a slot is to be empty, it has to be not set.
Proposed resolution
Allow empty array in slots. An empty Drupal render array should render to the empty string. The fix will require changing Drupal\Core\Render\Element::isRenderArray()
.
Remaining tasks
Agree on what to do.
User interface changes
None.
Introduced terminology
None.
API changes
Empty arrays will be allowed as the value for SDC slots.
Data model changes
None.
Release notes snippet
Issue fork drupal-3526176
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
sd9121 CreditAttribution: sd9121 at QED42 commentedComment #3
sd9121 CreditAttribution: sd9121 at QED42 commentedComment #4
idiaz.ronceroAgree that this is a bug. Especially if you consider that using an empty array using components directly on twig, you get no error at all: this only happens when using the RenderElement on your code.
There should be no difference on the accepted input values between the two ways to "invoke" a component.
Comment #6
idiaz.ronceroCreated a simple MR that allows empty arrays to bypass the negative check from
Element::isRenderArray
.This is a solution, but it might be controversial and is no small change as it assumes that,contrary to what Drupal usually does, an empty array will be considered a valid render array in this particular context.
A different solution could be the opposite: making the twig
embed
andinclude
s of components apply the same logic and disallow passing empty arrays as slot values (right now, they swallow it)What we have now - different behavior depending on the way a component is called - is misleading. We need to chose one direction or the other.
(I am obviously not proposing here to let developers pass empty arrays voluntarily, but you can end up with an empty array as an unintended result of some business logic, both on twig and on PHP code, and we need to decide if this is something we want to support or not).
Comment #7
liam morlandFix typo.
Comment #8
smustgrave CreditAttribution: smustgrave at Mobomo commentedComment #9
johnalbinYes, but, crucially, your MR isn't changing the logic of
Element::isRenderArray
so the definition of render array remains the same.ComponentElement
generates a Twig template on the fly and the MR modifies how/if the template is generated by checking if the contents of a slot are a render element and then creating a twig block containing the slot variable. IMO, I'd consider that a bug, because a twig block can contain any kind of content. Specifically, sinceComponentElement
is inserting a single variable (the slot's value) into a block, we should allow any variable value that Twig allows, not just render elements.Agreed. If we went the other way with the MR, then we would be limiting what is acceptable in Twig embed/includes; which would also be a breaking API change that would have to wait for 12.x. This MR expands what is acceptable for Single Directory Components and is not a breaking API change. And, as I said before, IMO, it's a bug fix. So this is the right way to go. 👍
The code looks good to my eyeballs, but as smustgrave pointed out, it needs tests.
Comment #11
oily CreditAttribution: oily as a volunteer and at Cactus Blossom IT Services Limited commentedTest coverage.