Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
barbieswimcrew opened this issue Mar 1, 2024 · 4 comments · Fixed by #1575
Closed

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

barbieswimcrew opened this issue Mar 1, 2024 · 4 comments · Fixed by #1575
Labels
Bug Bug Fix Status: Needs Review Needs to be reviewed

Comments

@barbieswimcrew
Copy link
Contributor

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 <twig:Alert /> 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 weaverryan added the Bug Bug Fix label Mar 1, 2024
@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Mar 1, 2024
@weaverryan
Copy link
Member

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

@kbond
Copy link
Member

kbond commented Mar 1, 2024

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
Copy link
Collaborator

WebMamba commented Mar 1, 2024

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

@kbond
Copy link
Member

kbond commented Mar 1, 2024

Yep, easiest fix for sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants