symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
805 stars 292 forks source link

[TwigComponent] Examples in the CVA documentation do not work #1570

Closed barbieswimcrew closed 5 months ago

barbieswimcrew commented 6 months ago

Hey guys, curious as I am, I just wanted to try out CVA. However, I'm running into problems:

An exception has been thrown during the rendering of a template ("Symfony\UX\TwigComponent\CVA::apply(): Argument #2 must be of type string, null given, called in /var/www/html/vendor/twig/twig/src/Extension/CoreExtension.php on line 1635").

That error happens when trying out CVA as documented here: https://symfony.com/bundles/ux-twig-component/current/index.html#component-with-complex-variants-cva

The error happens when I use and do not define any CSS classes. So, to me it looks like attributes.render('class') does not return an empty string but null. Am I doing something wrong or is the documentation incorrect?

{{ alert.apply({color, size}, attributes.render('class')|format) }} fixes the issue temporarily by casting the null to an empty string.

weaverryan commented 6 months ago

Looks legit to me! WDYT @kbond should attributes.render() always return a string? Or is there a better solution?

kbond commented 6 months ago

Three options here:

  1. Have attributes.render() always return a string - this would still work with |default() but would break ??
  2. Have CVA:apply() accept ?string ...$classes and filter out null/empty strings
  3. Both (1) and (2)

(2) is the safest and could help in general (imagine a completely different twig call that returns string|null)

WebMamba commented 6 months ago

Just made a PR for the solution 2. I think it's way much easier like that. Tell me what you think 😁

kbond commented 6 months ago

Yep, easiest fix for sure!