symfony / ux

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

[TwigComponent] data-model #555

Closed veewee closed 1 month ago

veewee commented 2 years ago

Hello,

We figured out that there is a data-model subscriber in twig components which allows you to:

<div>
    {{ component('textarea_component', {
        'data-model': 'value:someParentComponentProp'
    }) }}
</div>
<textarea>{{ value }}</textarea>

I was wondering: Why don't you just want to pass {value: someParentComponentProp} as the textarea_component attributes here?

The problem with the data-model here is that it interferes with the live components syntax of: norender|registration_form[password][first] or debounce(100)|someField.

This results in a

An exception has been thrown during the rendering of a template ("Can't get a way to read the property "norender|registration_form" in class "App\Twig\Component\SomeComponent".").

I currently don't see the value of the data-model subscriber and would suggest to remove it. But I'm sure you have a good counter argument for having it included.

norkunas commented 2 years ago

I got this error also. Thought that is something specific to my app, but looks like not..

weaverryan commented 1 year ago

I currently don't see the value of the data-model subscriber and would suggest to remove it. But I'm sure you have a good counter argument for having it included.

Indeed, the data-model syntax is unique because it means two things. For example, for 'data-model': 'value:someParentComponentProp', it means:

A) Pass a value prop to the child component B) When that value component is updated in the child, also update someParentComponentProp on the parent.

It's (B) that makes it unique vs just passing a value property manually.

But, I do think there there is a legitimate, unintended problem/ bug here :). You mentioned:

The problem with the data-model here is that it interferes with the live components syntax of: norender|registration_form[password][first] or debounce(100)|someField.

If I understand correctly, you're doing something like this?

<div>
    {{ component('textarea_component', {
        'data-model': 'debounce(100)|value:someParentComponentProp'
    }) }}
</div>

Is that right? To be honest, I hadn't considered that the child component could, itself, BE the field (e.g. textarea), but that's totally legitimate.

It seems to me that we just need the new data-model subscriber to be able to parse the debounce(100)|value... style of syntax. Or if someone has another idea, let me know!

veewee commented 1 year ago

The big problem here is that it is not consistent with how you define data-model inside live components. Inside live components, there is no targetProp:currentValue. This makes it impossible to pass down the live components version of data-model to a twig component.

I'm trying this:

<div>
    {{ component('textarea_component', {
        'data-model': 'debounce(100)|form[field]'
    }) }}
</div>

Where this data-model is configured in a symfony form through the attrs field.

The form is rendered inside a live component. The form control is rendered by a twig component, which passes down attrs from the sf form.

So either we need 2 data attributes or the otherwise the listener must be smart enough to skip data binding of it does not match the key:value syntax?


How does that 2-way bind (b) option work then? Because twig is rendered sequentially, it seems a bit odd. Can't seem to find an example of that case.

weaverryan commented 1 year ago

Back from the conference. And, yea, the problem is clear now. You are, very simply, wanting to do this:

<div>
    {{ component('textarea_component', {
        'data-model': 'debounce(100)|form[field]'
    }) }}
</div>

As a way to easily set the data-model attribute on the root element inside of that component. But now, I've given this data-model a new "meaning" for child components, which is conflicting. This is tricky, and I want to find the right solution.

How does that 2-way bind (b) option work then? Because twig is rendered sequentially, it seems a bit odd. Can't seem to find an example of that case.

The new behavior I've given to data-model for child components is modeled closely after Vue's v-model: https://vuejs.org/guide/components/events.html#usage-with-v-model - and it's only relevant for live components. The reason for this is described here: https://symfony.com/bundles/ux-live-component/current/index.html#updating-a-parent-model-from-a-child

To make things clear, here are the 2 possible/current "meanings" for passing data-model into a child component:

A) Example: 'data-model': 'debounce(100)|form[field]' - Please render this exact data-model HTML attribute -

B) Example: data-model: 'value:content' - Please (1) pass the content prop from the parent component into the child as a value prop AND (2) when that value prop changes, also update the content prop in the parent.

In Vue, their v-model does, indeed, have a different meaning when applied to an HTML element vs a custom component: their custom component usage equals what we have. Using that as wisdom, it suggests we should use meaning (B) for custom components and NOT (A).

If we did that (I'm thinking out loud), then how would we accomplish (A)? I think it would be with something like:

<div>
    {{ component('textarea_component', {
        name: 'form[field]'
    }) }}
</div>

Then in the component template:

<input data-model="debounce(100)|{{ name }}">

I know this is different than how things work currently. Live Components is still young, which is why I want to find the best solution vs just fixing what people have already. Thoughts?

veewee commented 1 year ago

Alright thanks for the in-depth explanation!

I'm still not convinced that this listener will be able to solve the issue. Imagine this nested scenario:

In this case, the markdown textarea won't be able to update the EditPost live component since it is nested in 2 other twig components. Even if it could, it will be done at some point in time when the markdown textarea is rendered and after the EditPost was mounted. This gives no way for doing e.g. validations in mount or set other props based on this new data.

If you do want to keep this kind of logic in the PHP listener, then I suggest following changes:

The first bullet will already resolve this specific issue we have and seems like a step that must be applied to make sure it only operates on your (B) type of model binding.

WDYT?

weaverryan commented 1 year ago

Thanks for continuing to iterate on this with me! I understand what you're saying, but before considering one of those options, could you describe your full use-case and what you want to accomplish? I think you have a LiveComponent that uses ComponentWithFormTrait. And I also think that, for at least some fields, you render those in another Twig (not live) component - that's the textarea_component. How & where are you doing that? Do you have a reusable textarea_component that you reuse in various forms?

Same question for you @norkunas - you mentioned that you were also affected, I'd love to hear exactly how you were/are currently use data-model and what you're accomplishing with it.

veewee commented 1 year ago

The use-case is pretty much described in #566. We have a live component that shows a form through the ComponentWithFormTrait. Inside this form we want to control if specific fields should trigger a rerender or not. This way we can build a form with dynamic elements. in order to achieve this, we need to set the data-model to 'norender'. But the twig component listener is blocking us to do so.

In our codebase, we introduced an atomic design with twig components. More info: https://bradfrost.com/blog/post/atomic-web-design/

Basically, we have atoms for buttons, inputs, checkboxes, selects, ... which we can compose troughout the application. This allows us to do very precise tailwind styling on those components and use them as standalone components in twig. To make sure it works with SF forms as well, we introduced a custom SF form twig theme that renders these twig "atom" components under the hood. Attributes from SF forms are passed down to the custom twig components, including the data-model attribute. This is what is making the error appear. Yet the real issue is the listener not being smart enough atm.

I hope this explanation is clear to you. If not, we are open to plan in a call so that we can showcase a demo to you.

The error here is not solely related to the form trait, but rather with the rendering of components because if the listener.

norkunas commented 1 year ago

@weaverryan my use case is pretty simple - I wanted to apply 'data-model': 'on(change)|*' rule on a form, which is in a component.

page template:

{% block body %}
{{ component('Search') }}
{% endblock %}

component template:


<div{{ attributes }}>
{% component Paper with { element: 'form', method: 'GET', 'data-turbo': 'false', 'data-controller': 'search', 'novalidate': '', 'data-model': 'on(change)|*' } %}
  {% block children %}
    live form inputs..
  {% endblock %}
{% endcomponent %}
</div>
weaverryan commented 1 year ago

@veewee This helps! And very cool use-case. You mentioned:

Attributes from SF forms are passed down to the custom twig components, including the data-model attribute. This is what is making the error appear. Yet the real issue is the listener not being smart enough atm.

So the current flow is:

A) You set the data-model in the form type, like mentioned in #566

    ->add('email', EmailType::class, [
        'required' => true,
        'attr' => ['data-model' => 'norender|registration_form[email]'],
    ])

B) You have a custom form theme (for EmailType in this case?) that renders a component. Is it something like this?

{% block email_widget %}
    {{ component('email_form_field', attr)
{% endblock %}

C) Then, inside the component template, you're rendering the field by "hand" in some way - e.g.

<input {{ attributes }} type="email">

Is this the basic idea? And can you tell me where I've got something wrong? I really want to "grok" the full use-case so we can find the coolest path forward 😎

@norkunas Ah. Is that Paper component is a ComponentWithFormTrait component? Or does it contain a manual form via HTML? I guess it looks something like this perhaps?

<form {{ attributes }}>
    {% block children %}{% endblock %}
</form>

And, if so, is the Search component one that uses ComponentWithFormTrait? And if it does, shouldn't the data-action be added to the form via that component? And if it does not use ComponentWithFormTrait, could you, instead of adding data-model to the <form, add data-model to the individual form fields? Does this form both re-render via LiveComponents and also submit to some route/controller?

veewee commented 1 year ago

@weaverryan Exactly this!

But again, the same error applies if you leave out the ComponentWithFromTrait, since it is a listener on twig components - even though this logic only applies for live components.

weaverryan commented 1 year ago

What about you @norkunas? I was hoping to get some more details on your use-case:

@norkunas Ah. Is that Paper component is a ComponentWithFormTrait component? Or does it contain a manual form via HTML? I guess it looks something like this perhaps?

<form {{ attributes }}>
    {% block children %}{% endblock %}
</form>

And, if so, is the Search component one that uses ComponentWithFormTrait? And if it does, shouldn't the data-action be added to the form via that component? And if it does not use ComponentWithFormTrait, could you, instead of adding data-model to the <form, add data-model to the individual form fields? Does this form both re-render via LiveComponents and also submit to some route/controller?

@veewee btw, even though this makes no sense in twig components land, I want to apply whatever the final logic is consistently so that if you change a twig component to a live component, you don't get behavior changes. We'll find something that works nicely :).

norkunas commented 1 year ago

@norkunas Ah. Is that Paper component is a ComponentWithFormTrait component? Or does it contain a manual form via HTML? I guess it looks something like this perhaps?

<form {{ attributes }}>
    {% block children %}{% endblock %}
</form>

And, if so, is the Search component one that uses ComponentWithFormTrait? And if it does, shouldn't the data-action be added to the form via that component? And if it does not use ComponentWithFormTrait, could you, instead of adding data-model to the <form, add data-model to the individual form fields? Does this form both re-render via LiveComponents and also submit to some route/controller?

Yes it's a live search form and uses that trait (Paper is just to wrap all form elements into it while also changing from 'div' to 'form'). Adding to the individual fields is the thing that I wanted to avoid, and the docs says that its enough to add this to the form element, and this was working before :)

carsonbot commented 6 months ago

Hey, thanks for your report! There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

carsonbot commented 6 months ago

Friendly ping? Should this still be open? I will close if I don't hear anything.

carsonbot commented 5 months ago

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

sheepwall commented 1 month ago

Scratch that, I messed up. Sorry for bumping this.