- Issue created by @Liam Morland
- 🇪🇸Spain idiaz.roncero Madrid
Agree 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.
- Merge request !12356Consider empty arrays a valid renderable on ComponentElement → (Open) created by idiaz.roncero
- 🇪🇸Spain idiaz.roncero Madrid
Created 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).
- 🇹🇼Taiwan johnalbin Taipei, Taiwan
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.
Yes, 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.A different solution could be the opposite: making the twig embed and includes 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.
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.
- First commit to issue fork.