[LiveComponent] Support DataModel bindings with LiveComponent modifiers

[Wertisdk]

Q	A
Bug fix?	yes
New feature?	no
Deprecations?	no
Documentation?	no
Issues	Fix #3152, Fix #555
License	MIT

[LiveComponent]

Support DataModel bindings with LiveComponent modifiers

The data-model attribute is used by both the Data Binding (for parent-child prop binding) and LiveComponent (for form input binding with modifiers). This was causing LiveComponent modifiers like 'norender|field' or 'debounce(100)|field' to fail because the property was not extracted from the LiveComponent data-model attribute, but was used directly as a property

This fix adds modifier detection to DataModelPropsSubscriber that checks if data-model contains LiveComponent modifier syntax (pipe '|') and extracts the value from the modifier syntax

Co-developed with @mrbase

[Wertisdk]

I am unsure what the failed test signifies and what could be the cause:
Unit Tests / php (8.1, lowest) (pull_request)

[mykiwi]

@smnandre @Kocal Any idea when this could be released?

[Wertisdk]

@smnandre please let me know if there's anything I can do to address the failed unit test or any other feedback for this to be accepted 😃

[smnandre]

Well if I don't send my review..... 🤦‍♂️ sorry for that

[smnandre]

Hmm.. was this mix of the two syntaxes ever supported by LiveComponent ? I mean, documented etc ?

If not (what i'd say but with not certainety) this is not a bug fix but a feature :)

And one i fear I would be very critical of... as it creates (another) layer of confusion for people.

Do you think there any way we can solve your original core need, that does not require to add another degree of complexity here ?

[Wertisdk]

@smnandre Do you mean

1. specifically the norender|content:value syntax where both '|' and ':' is used together or
2. the concept in general of supporting modifiers in the data-model attribute when added as an attribute in the component method?

I could fix 1. by checking that ':' is not present before trying to to extract the property name and amending the test to expect an exception thrown because the property is not present (because it is being returned literally and did not match a property).

[smnandre]

More "2" here : I had the impression data-model was either used for child component using the xx:yy syntax, or into single components with modifiers, but not both.

[Wertisdk]

Thanks a lot for taking the time to look at this!

I completely understand the concern about adding complexity, and I also agree that if mixing the two syntaxes was never intended, then treating this as a feature rather than a bugfix is reasonable.

The main reason we experienced this as a bug is not so much the specific behavior, but the inconsistency that emerges from reusing the same data-model attribute in both contexts.

If I write

<select data-model="norender|myProperty">...</select>

this works exactly as documented. But if I wrap that same <select> as the root element of a Twig component and use:

<twig:MySelectComponent data-model="norender|myProperty" />

... or ... 

{{ component('MySelectComponent', {
    dataModel: 'content',
}) }}

the exact same syntax suddenly fails, even though from my perspective it feels like the same operation, just composed.

This is where my surprise comes from: not that modifiers exist, but that the same attribute value behaves differently depending on whether it’s applied directly to a DOM element or passed through a Twig component. If these two concepts were never meant to interact, then reusing data-model for both makes that distinction very hard to discover or reason about as a developer.

Since the different behaviour is not explicitly mentioned in the documentation, I believed this was a bug.

That said, I absolutely defer to the project’s direction here. If it's preferred to not support modifiers in Twig component bindings, I think documenting that limitation (or even failing fast with a clearer error) would already help a lot.
The approach in the PR felt like the least intrusive way to align the behavior with user expectations.

We can work around this issue in a couple of ways, one of them by manually adding modifiers in our Twig component like I describe in #3152 (comment)

Happy to adjust the PR in whatever direction makes the most sense for Symfony UX or contributing to explicitly documenting the behavior, and thanks again for the guidance! :)

[Wertisdk]

@smnandre
I see now, that @carsonbot went a bit overboard with labels, because I initially had a formating error in the table for the PR...
Is there something I can do to make @carsonbot reevaluate the labels, or do someone need to fix them for me? :)

[Wertisdk]

@smnandre Sorry to mention you, but is there anything else I should do with this pull request after you changed the labels?
I am new to this project, so I just want to make sure I'm not missing a process I should be aware of :)