symfony / ux

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

[LiveComponent] null argument is filled as empty string #1691

Closed jannes-io closed 5 months ago

jannes-io commented 5 months ago

Strange DX when using nullable types in LiveAction.

Using the following LiveComponent:

#[AsLiveComponent('TestComponent', 'test_component.html.twig')]
class TestComponent
{
    use DefaultActionTrait;

    #[LiveAction]
    public function action(#[LiveArg] ?int $id): void
    {
        //...
    }
}

With the following template:

<div {{ attributes }}>
    <a
        data-action="live#action"
        data-live-action-param="action"
        data-live-id-param="{{ null }}"
    >
        Click me!
    </a>
</div>

Expected: The function is executed with $id === null

Current: TypeError: "string" cannot be assigned to "int or "null".

Could we maybe include some sort of type transformation that when a LiveArg is nullable, and the accepted types does not include string, and an empty string is passed in, the value is set to null?

Passing in the string "null" to the param, it does coerce it to null. Which just results in weird DX imo. Real world example:

<a
    data-action="live#action"
    data-live-action-param="setCreating"
    data-live-parent-id-param="{{ parentId is not null ? parentId : 'null' }}"
>

Could we take a look at this behavior and perhaps tweak it so we don't need to cast null to string "null" when using nullable types on LiveAction's LiveArgs?

WebMamba commented 5 months ago

Yes, just try locally and see your point now. It sounds like a bug, are you up to a PR?

smnandre commented 5 months ago

Hmm.. it's the same debate on the symfony/symfony repo ... it's not that obvious that empty string and null are the same thing :|

jannes-io commented 5 months ago

are you up to a PR?

@WebMamba sure, I’ll take a look first thing tomorrow. Since LC is stable now it’ll be important to take BC into account with empty string values, so we can’t just blindly set empty strings to null.

jannes-io commented 5 months ago

@smnandre , @WebMamba , created a proposal PR. Let me know what you guys think. Imho it's a bit convoluted since we need to keep actual empty strings into account and can't just blindly convert.

Thanks.