symfony / ux

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

[LiveComponent] Can not update parent component with array of DTOs #1953

Open cgrabenstein opened 1 week ago

cgrabenstein commented 1 week ago

I started working with Live Components and noticed the following behaviour:

Imagine having a ParentComponent and a ChildComponent. ParentComponent holds an array of DTOs like that:

#[AsLiveComponent]
final class ParentComponent
{
    use DefaultActionTrait;

    /**
     * @var IntWrapper[]
     */
    #[LiveProp(writable: true)]
    public array $values = [];

    public function mount(): void
    {
        $one = new IntWrapper();
        $one->value = 1;

        $two = new IntWrapper();
        $two->value = 2;

        $three = new IntWrapper();
        $three->value = 3;

        $this->values = [$one, $two, $three];
    }
}

The ChildComponent holds a single instance of the DTO:

#[AsLiveComponent]
final class ChildComponent
{
    use DefaultActionTrait;

    #[LiveProp(writable: ["value"])]
    public IntWrapper $value;

    public function mount(): void
    {
        $this->value = new IntWrapper();
        $this->value->value = 10;
    }
}

I then render the ParentComponent with the ChildComponent inside:

ParentComponent.html.twig

<div {{ attributes }}>
    {% for key, val in values %}
        <span>{{ val.value }}</span>
        <twig:ChildComponent data-model="values[{{ key }}].value:value.value"/>
    {% endfor %}
</div>

ChildComponent.html.twig

<div {{ attributes }}>
    <span>nested:</span> <input type="number" data-model="value.value">
</div>

By binding values[{{ key }}].value to value.value I expected the ParentComponent to update, when I change the value in the input field of the ChildComponent, as described in the docs.

However the value in the ParentComponent is not updated. The problem is in the https://github.com/symfony/ux/blob/2.x/src/LiveComponent/src/LiveComponentHydrator.php#L365 where the property path is adjusted before setting the updated value.

In the presented example, the expected output of the adjustPropertyPathForData would be "[0].value" since we are changing a property of an object inside an array. However the actual output is "[0][value]". The $currentValue inside the method is initially an array, but the value is not being updated as the foreach-loop processes the parts of the property path.

I'm not sure if this is a bug, or if our use case is just not (yet) possible with the current state of Live Components. Possible workarounds are either working with arrays only, without using DTOs, or implementing \ArrayAccess on our DTOs.

I hope I could make my issue clear, please let me know what you think about it.

smnandre commented 6 days ago

Do you need those child to be live component ? Could they not be directly properties of the parent one ?

cgrabenstein commented 6 days ago

That's actually a good question. In this example, sure. But it is also dramatically simplified. In our application we have a list with multiple items, similar to this example, but with an DTO in the ItemComponent, instead of the distinct properties.

Again, we could probably remove the ItemComponent all together, but then I'm asking myself what are good heuristics whether to create a separate (child) Component? Is it reusability? Is it a certain number of properties? Specific behaviour? I guess what I am still lacking is the intuition on how to structure the components, how to split or merge them.

Back to the issue at hand, the following steps are all working fine:

So the only "faulty" piece is in the adjustPropertyPathForData method, where the $currentValue is not updated while traversing the property path parts.

cgrabenstein commented 5 days ago

After some more debugging I realised that this has nothing to do with parent and child. Even if I drop the child component completely, I am not able to update the DTO in the array. The property path to update is still 0.value which will turn into [0][value] in the adjustPropertyPathForData method. And the exception Cannot modify index "value" in object of type "IntWrapper" because it doesn't implement \ArrayAccess. will be thrown nevertheless.

smnandre commented 5 days ago

Could you share your IntWrapper code ?

Have you tried using a custom hydrateWith / dehydrateWith methods ? Or use the serializer ?

cgrabenstein commented 5 days ago

Here is a demo project I just created: https://github.com/cgrabenstein/symfony-ux-demo

Note how the single DTO is updating correctly, but the DTOs in the array do not update. You can add a dump($exception) in vendor/symfony/ux-live-component/src/LiveComponentHydrator.php:412 to get the exception I mentioned above. This is not related to hydration as far as I can tell.

smnandre commented 4 days ago

Is there any chance using only name notation could help (by miracle) ?

https://github.com/symfony/ux/issues/1851#issuecomment-2181196027

cgrabenstein commented 3 days ago

No, I tried it, but it does not help. The code in your linked comment uses an array of arrays. I am using an array of objects. The method adjustPropertyPathForData does not support array of objects.

smnandre commented 3 days ago

So that mean you should probably try with a collection ... but i'm 40% sure (no more i admit 😅 ) we had some way to handle this :/