ux icon indicating copy to clipboard operation
ux copied to clipboard

[LiveComponent] Using form without Entities

Open janklan opened this issue 3 years ago • 4 comments

In my application, my forms don't manipulate Entities. My forms manipulate DTO classes. Once submitted, validated and accepted, those DTO classes are passed to methods in relevant entities to mutate them according to the DTO contents.

It has a ton of benefits, but it also revealed some sort of tight coupling between a mutable Doctrine entity, and a form living in a LievComponent when I tried to build a LiveComponent around my forms. It looks like there is a lot of magic happening behind the sceens. I think it's worth getting to the bottom - less magic = more predictability & flexibility.

I constructed a minimal PoC of the DTO-driven architecture I described above, and I built a standard form to demo the code works, and a LiveComponent. The LiveComponent currently does not work, because when the Form is being rendered, the magician fails to denormalise the form data into my DTO class.

  1. (src/Entity/Post.php)[https://github.com/janklan/symfony-ux-playground/blob/dto/src/Entity/Post.php] is the main entity I am using to demo. It belongs to an Author, and has a ManyToMany relation to Tags. Notice all Post props are private, only getters, and then there are two methods at the bottom. One can build a new entity, one can mutate an existing entity. These are the benefits I'm talking about: predictable consistency, concealed internal state.
  2. (src/Dto/PostUpdateDto.php)[https://github.com/janklan/symfony-ux-playground/blob/dto/src/Dto/PostUpdateDto.php] is the DTO created from an existing Post, and then (updated once the form is submitted and validated)[https://github.com/janklan/symfony-ux-playground/blob/dto/src/Controller/DefaultController.php#L98-L108]
  3. Check out the repo, follow the few instructions and get it running. You'll see that at https://127.0.0.1:8000/edit/1?standard=1 shows the standard edit form that proves that things outside LiveComponent work. A gif will be below.
  4. https://127.0.0.1:8000/edit/1?standard=0 is the LiveComponent, which should accept both PostCreateDto|PostUpdateDto - in fact, it should accept CreateDtoInterface|UpdtateDtoInterface, which I'm using in my real-world architecture for all DTO, but I didn't want to complicate things here. The failure takes place
  5. The LiveComponent first renders correctly, but on the first reload fails, because it's trying to set the Form data array to my public PostCreateDto|PostUpdateDto|null $dto;
  6. When I (change the $dto to public ?PostUpdateDto $dto = null;)[https://github.com/janklan/symfony-ux-playground/blob/dto/src/Twig/Components/PostFormComponent.php#L25-L26], the LiveComponent magically starts working. I need to be able to type hint the DTO dynamically, otherwise I have to create a lot of duplicate components, because I have lots of different forms with lots of different entities.
image

Is there any way to give the Serializer/LiveComponent/etc machinery a hint of what should the data object actually be?

I tried to use the hooks and mount() method, but they works as the docs say - on the first render. The re-render seems to be completely out of our control?


The gif recording for item 3 - the form works when outside a LiveComponent:

2022-08-08 17 18 03

The gif recording for item 4 - the form fails when inside a LiveComponent:

2022-08-08 17 18 34

janklan avatar Aug 08 '22 07:08 janklan

Hi @janklan!

Thanks for the detailed issue and reproducer - the reproducer was super high quality - that's a great form you built! I got it running locally to help out.

So, the problem, as you detailed, is the UnionType. You have:

#[LiveProp(fieldName: 'data')]
public PostCreateDto|PostUpdateDto|null $dto;

That's simply not something that we've supported yet. Right now, if you have just ONE type (e.g. PostUpdateDto), then we pass the input data through Symfony's denormalizer and tell it to denormalize to that class. But right now, we don't handle the union type at all... so we just set the raw data (in this case an array) onto the property, which causes the explosion.

When you're using union types, live components can't, of course, guess which to use: you'll need to tell it. It might make sense to have a new "type" option to explicitly control the type:

#[LiveProp(fieldName: 'data', type: PostCreateDto::class)]
public PostCreateDto|PostUpdateDto|null $dto;

However, I'm not sure this helps you? Because you would need this type to be dynamic, right? How would that work in a perfect world? We could SEE that you originally passed a PostCreateDto and then keep it as a PostCreateDtoon re-render. But to do that, we would need to pass around theApp\Dto\PostCreateDto` string in the Ajax data... which seems ugly: ideally the "typing" stays in the code.

Another solution that would work right now, though the DX may not be super great (we'll have to see) is to use the hydrateWith option:

#[LiveProp(fieldName: 'data', hydrateWith: hydrateDto)]
public PostCreateDto|PostUpdateDto|null $dto;

public function hydrateDto($data)
{
    // $data would in practice be an array in your case
    // then you would return the PostCreateDto or PostUpdateDto
    // though... I'm not sure how you would determine which situation you are in at this point
}

Let me know what you think :)

weaverryan avatar Aug 09 '22 16:08 weaverryan

Hi @janklan!

hey! :)

So, the problem, as you detailed, is the UnionType... ... when you're using union types, live components can't, of course, guess which to use: you'll need to tell it.

That's precisely the problem, and I understand where it is coming from and agree with reasons it works like that.

In my real world, it gets even more complicated. I didn't include it in my reproducer, but all my DTOs implement UpdateDtoInterface and CreateDtoInterface, and I have a service that understands those interfaces and handles data validation and persisting so I can use those DTOs outside the form context (which does the validation for me).

So ultimately the LiveProp union type won't even be PostCreateDto|PostUpdateDto, but CreateDtoInterface|UpdateDtoInterface. Just to make things more complicated, right?!

#[LiveProp(fieldName: 'data', type: PostCreateDto::class)]
public PostCreateDto|PostUpdateDto|null $dto;

However, I'm not sure this helps you? Because you would need this type to be dynamic, right? How would that work in a perfect world? We could SEE that you originally passed a PostCreateDto and then keep it as a PostCreateDtoon re-render. But to do that, we would need to pass around theApp\Dto\PostCreateDto` string in the Ajax data... which seems ugly: ideally the "typing" stays in the code.

Correct, that doesn't help either. Moreover, some DTOs are created with some properties that are not even part of the form but are needed later on to build/mutate the final entity. Check this real-world example code from a standard controller. Check the few inline comments I added.

#[Route('/{project<\d+>}/task/create/{employee<\d+>?}/{date?}', name: self::ROUTE_CREATE)]
public function create(Request $request, EntityService $entityService, Project $project, ?Employee $employee, ?\DateTimeImmutable $date, HubInterface $hub, WorkflowInterface $taskStateMachine): Response
{
    $form = $this->createForm(
        TaskCreateType::class,
        new TaskCreateDto(
            project: $project, // The project is part of the DTO, but not part of the form. 
            requestedBy: $this->getUser(),
            startDate: $date ?: new \DateTimeImmutable(),
            assignedTo: $employee,
        )
    );

    $form->handleRequest($request);

    if ($form->isSubmitted() && $form->isValid()) {

        // To be truly universal, the Entity Service requires DTOs to be fully insulated - they have to contain all the information needed to build/mutate their entities, hence the `$project` stored in the DTO
        $task = $entityService->create($form->getData(), Task::class);

        $taskStateMachine->apply($task, TaskTransition::CREATE);

        $entityService->flush();

        $hub->publish(new Update(
            'activity-planner-team-'.$task->getAssignedTo()->getTeam()?->getId(),
            $this->renderView('task/create.stream.html.twig', ['task' => $task]),
        ));

        return $this->redirectToRoute(self::ROUTE_READ, [
            'task' => $task->getId(),
        ]);
    }

    return $this->renderForm('task/create.html.twig', [
        'form' => $form,
    ]);
}

The Task creator then simply calls $this->project = $dto->project. So this will never work under the current LiveComponent circumstances because its built-in API endpoint has no idea the DTO is in place. Instead, the endpoint works with the array provided by the form and relies on the serializer to work things out, where it all falls apart.

Another solution that would work right now, though the DX may not be super great (we'll have to see) is to use the hydrateWith option:

#[LiveProp(fieldName: 'data', hydrateWith: hydrateDto)]
public PostCreateDto|PostUpdateDto|null $dto;

public function hydrateDto($data)
{
    // $data would in practice be an array in your case
    // then you would return the PostCreateDto or PostUpdateDto
    // though... I'm not sure how you would determine which situation you are in at this point
}

I tried that, but at that point, not even the EntityType field values are converted to entities etc., so I (1) was forced to properly understand how the Serializer does its magic, and (2) abandoned it thinking there must be a better way.

So the problem here is that when LiveComponentSubscriber decides a request is a Live Component request, it interrupts the flow, does it's magic, and returns the response.

It feels pretty similar to my other issue with the custom autocompleter endpoint we discussed elsewhere - this use case is probably both common and too complicated to try to support natively.

The root cause is that LiveComponentSubscriber doesn't have the same information at hand as what the controller has when using #[Route('/{project<\d+>}/task/create/{employee<\d+>?}/{date?}', name: self::ROUTE_CREATE)]. I build and provide the DTO to the form, removing the need to do any guesswork the LiveComponentSubscriber has to do now.

So let me provide the context:

  1. If I was able to instruct the LiveComponent which URL it should be talking to, I could maybe re-render the component myself in Twig, much like when I did at the beginning of the whole process with {{ component('form', {...}) }}. I checked the LiveComponentSubscriber, and it does do some magic in createResponse, but it's not that bad. Maybe we can move the response generation from the subscriber to a service I could autowire into my controller action, build the context around the LiveComponent and then call that service to build that response?

  2. The subscriber could fire an event around the time the component is being built, and I could listen to that event and provide the missing context - the initial DTO itself. Probably sometime before the LiveComponentHydrator does its job?


As conclusion, I think there is a case for re-using a live component to support forms universally. We're already passing all the custom-made objects: view, form, data. If I had to create a component for each form, the only thing that would differ is the type hint on my DTO live prop. It sounds like we're using the type int as a runtime configuration hint.

I'm keen to hear your thoughts.

And thanks - this is all great and is heading the right way!

janklan avatar Aug 09 '22 22:08 janklan

@weaverryan I had another go. I tried to create a base LiveComponent for forms and extend it to provide the individual type hints to minimise the code duplication, but my child live components were not even recognised. I'm not sure why and I didn't dig into it (and I don't have a reproducer this time), but when I cut&pasted all the code from my extending component back up to the parent, added the #[AsLiveComponent], it started working. Something possibly off with the attributes processing?

Anyway, that's not why I'm back here. I tripped over additional few other problems with a real-world scenario form - a few EntityTypes that show and hide following checkbox state changes etc.

I've been tripping over serialisation issues in trivial scenarios, such as when I ticked second EntityType checkbox etc:

2022-08-11 07 00 05

In the gif above I got Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader::getIdValue(): Argument #1 ($object) must be of type ?object, int given.

I am thinking: all this is a non-issue when using the same form without the live component. The problem therefore must be created by that universal API endpoint that doesn't know what to do with the data. So if the component talked to my own endpoint, where I can provide the right context, there would be no further magic needed on the component's end.

In other words, I slept on what I wrote yesterday, and I think the suggestion no .1 at the end of the comment is a valid solution.

janklan avatar Aug 10 '22 21:08 janklan

I'm not sure why and I didn't dig into it (and I don't have a reproducer this time), but when I cut&pasted all the code from my extending component back up to the parent, added the #[AsLiveComponent], it started working. Something possibly off with the attributes processing?

That is very possible - I'd have to see some example code, but I haven't tried live components with extension yet, so I'm not entirely sure how the attribute would be parsed.

I am thinking: all this is a non-issue when using the same form without the live component. The problem therefore must be created by that universal API endpoint that doesn't know what to do with the data. So if the component talked to my own endpoint, where I can provide the right context, there would be no further magic needed on the component's end.

I'm not completely convinced - this could also be that the data isn't being sent (by the frontend) in the correct format. You might have a point, I'm just not convinced yet. In theory, on the frontend, we are trying to collect the form data in the same way that if you submitted the data. We send that data up to the endpoint and "submit" the form. I don't think there's any complex deserialization happening here: the "raw" form values are kept as an array https://github.com/symfony/ux/blob/5893aca1b239a48b97b09fc1c6316b65f9d2d749/src/LiveComponent/src/ComponentWithFormTrait.php#L45

If you're able to get a reproducer, that'd be super helpful for digging into this.

The root cause is that LiveComponentSubscriber doesn't have the same information at hand as what the controller has when using #[Route('/{project<\d+>}/task/create/{employee<\d+>?}/{date?}', name: self::ROUTE_CREATE)]. I build and provide the DTO to the form, removing the need to do any guesswork the LiveComponentSubscriber has to do now.

That's true. But couldn't any of this extra data be put onto a LiveProp? Then you could use the hydrateWith attribute option to call a custom method where you use the extra information to create the Dto exactly how you want? There are a lot of details flying around, so I may be missing some items. But the point is: you are free to store whatever data you might want as a LiveProp (so this takes the place of the {wildcards} you might normally have in a route. We may still be missing some flexibility to get this whole thing working, but we need to find what those things are - I don't see a need for this route idea yet.

Cheers!

weaverryan avatar Aug 11 '22 20:08 weaverryan

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

carsonbot avatar Apr 26 '24 12:04 carsonbot

No and given Ryan's context and my current focus on other things, I don't expect it to do so in a hurry.

That doesn't mean the use case and its problems are no more.

janklan avatar Apr 26 '24 21:04 janklan

I'm closing this one (for the reasons ...)

Feel free (really) to open another issue if you want :)

smnandre avatar Apr 26 '24 22:04 smnandre

Huh, what's the point of closing this and telling me to open another issue?

janklan avatar Apr 26 '24 22:04 janklan