[Twig] Add attribute merging behaviour

[kbond]

Q	A
Bug fix?	no
New feature?	yes
Issues	n/a
License	MIT

Currently, when using attribute defaults, the merging behaviour of individual attribute values is a bit arbitrary, class, data-controller and data-action keep the default value as the prefix but any other completely overrides the default:

{# MyComponent #}
<div {{ attributes.defaults({
    class: 'default',
    style: 'display:block;',
})>

{# render #}
<twig:MyComponent class="new" style="margin: 0;" />

{# output #}
<div
    class="default new"
    style="margin: 0;"
>

This PR proposes an attribute value prefix notation to configure the behaviour:

• ^: - prefix
• $: - suffix
• @: - replace (default except for the exceptions above - this allows configuring these exceptions)

Suggestions on a better/alternate notation or even an alternate way to acheive is welcome!

Usage:

{# MyComponent #}
<div {{ attributes.defaults({
    class: 'default',
    style: 'display:block;',
    'data-something': 'foo',
})>

{# render #}
<twig:MyComponent class="@:new" style="$:margin: 0;" data-something="^:bar" />

{# output #}
<div
    class="new"
    style="display:block; margin: 0;"
    data-something="bar foo"
>

I wish we could remove the special exceptions but not sure the best way to do this. We could deprecate not passing a behaviour for these. And set their default to "replace" in 3.0 but this might be heavy handed.

[smnandre]

This one make me a bit anxious... as it's kinda a BC break... but i know how many of you want this feature out :)

i'm wondering if we should have the "mode" supported by the attribute key and not the value.

class+=

class=+

class=

Does this work only on declared attributes ? Or all ? In wich case this could create strange behaviour with mount / events no ? :|

[kbond]

as it's kinda a BC break

How so? Only thing I could think of is someone using this type of notation already for something else so I tried to choose something very obscure.

i'm wondering if we should have the "mode" supported by the attribute key and not the value.

Do you mean something like?

<twig:Component class@="replace" class^="prefix" class$="suffix" />

Quick test shows that some changes would be needed in the ComponentLexer.

Does this work only on declared attributes ? Or all ? In wich case this could create strange behaviour with mount / events no ? :|

I could be missing something but I can't see how it could affect anything else - this is all internal to ComponentAttributes.

[smnandre]

I mean there is already a lot of different things and usage for those values... When you type class="$:..." you have no idea if it's a mounted property, an attribute, etc etc.

So i'm a bit mixed for now, as I really feel there are great features coming (and it's super cool)... but it may happen people don't have in mind all the interactions between twig component / live component / .... and all the ways data is handled / displayed / saved.

So not especially for this one, more a general feeling we should isolate things a bit.

--

Concerning those evolutions i still believe i'll want to protect some PHP attribute / use some readonly, but that absolutely can be done later (and by me if i'm the one asking for it haha)

--

So let's focus on your PR for now :)

[weaverryan]

<twig:Component class@="replace" class^="prefix" class$="suffix" />

I think this is a no-go, even though I did like the idea of class+= and class=+. The problem is that this no longer looks like a class attribute to editors... which I think is just going to make things weird. Even adding the merge behavior inside the value is going to look odd to editors - class="+:hidden". Would Tailwind even parse that correctly?

I think, whatever the solution, it is going to need to live inside the component itself so that the usage can remain as normal HTML.

[kbond]

Agree that adjusting the html syntax would be difficult and wouldn't work with non-html syntax components.

Even adding the merge behavior inside the value is going to look odd to editors - class="+:hidden". Would Tailwind even parse that correctly?

The end html sent to the browser wouldn't include this syntax - it's parsed out. Oh I guess you meant in development. Yeah, I'll have to check on this.

[kbond]

Note, in its current form, only when rendering the component can you use this notation. You cannot use it in {{ attributes.defaults(...) }}. Based on some comments in Ryan's last livestream. Adding to defaults would also be desired (maybe preferred).

[weaverryan]

What about this syntax?

<div
    class=“{{ attributes.class }} foo bar”
    should-override=“{{ attributes.get(‘should-override)|default(‘default-val’) }}
    {{ attributes }}
>

It would work like the form system: as you reference attributes like “class”, they’re marked as rendered. At the end, it renders anything remaining. attributes.class is a shortcut for attributes.get(‘class’).

This gives users full control over merge behavior, you could use filters like tailwind_merge and it allows you to write native HTML attributes. The only gotcha would be to know not to write class”foo” without remembering to also render {{ foo }} else you’ll end up with 2x class attributes.

[kbond]

This gives users full control over merge behavior

But only from inside the component, correct? You couldn't choose the merge behavior when rendering a component (maybe this isn't a realistic scenario as someone pointed out in the livestream chat).

What about this syntax?

In theory, I like this. It would require ComponentAttributes to no longer be immutable. But seeing that get() would be a new method, I don't think there would be a BC break.

[smnandre]

I'm 100% for something decided on the "render" side and not the "calling" side.

[kbond]

I'm 100% for something decided on the "render" side and not the "calling" side.

Ok, that gives us more options for sure - something like:

{# MyComponent #}
<div{{ attributes
    .defaultPrepend('data-controller', 'default')
    .defaultAppend('style', 'default')
    .defaultReplace('class', 'default')
}} />

{# render #}

<twig:MyComponent data-controller="foo" style="bar" class="baz" />

{# html #}

<div data-controller="default foo" style="bar default" class="baz" />

[weaverryan]

Do you like that version better than my “pure attributes” version? I’m kinda solving the prepend problem AND the syntax in one go… if we like that one.

it also allows for things like tailwind merge. It feels a bit better to just be echoing things vs configuring and object perfectly so that once it is printed in its entirety, it prints correctly. But I know we borrowed the current implementation from blade, so it’s possible my way is not considering some case.

[kbond]

Do you like that version better than my “pure attributes” version?

Not necessarily better, I was really just pointing out that only allowing controlling the behaviour in the component allows us to remove the weird syntax I came up with.

it also allows for things like tailwind merge.

You're right, it makes that easier. Is this what you have in mind?

<div
    class="{{ (attributes.class ~ " foo bar")|tailwind_merge }}"
>

That's a bit verbose but I'm sure it could be improved/simplified. Maybe:

<div
    class="{{ attributes.class.append('foo bar')|tailwind_merge }}"
    class="{{ attributes.class.prepend('foo bar')|tailwind_merge }}"
    class="{{ attributes.class.default('foo bar')|tailwind_merge }}"
>

[weaverryan]

Yup! That’s what I had in mind :). Though running an attribute where you need to merge with defaults and use a filter is an edge case (and I still think we could tailwind merge on class automatically). So I wouldn’t optimize for that case. But if these .prepend style functions are optional (meaning, you can just do class=“attributes.class foo bar), then I think we may be in business.

[kbond]

But if these .prepend style functions are optional

Yep, calling get() would return a stringable object

then I think we may be in business.

You're ok with breaking ComponentAttribute's immutability?

Edit: actually, could that be a problem for live components? If we unset the values, they won't be available on re-render? I think it would be but I'll have to confirm

[CMH-Benny]

What about this syntax?

<div
    class=“{{ attributes.class }} foo bar”
    should-override=“{{ attributes.get(‘should-override)|default(‘default-val’) }}
    {{ attributes }}
>

It would work like the form system: as you reference attributes like “class”, they’re marked as rendered. At the end, it renders anything remaining. attributes.class is a shortcut for attributes.get(‘class’).

This gives users full control over merge behavior, you could use filters like tailwind_merge and it allows you to write native HTML attributes. The only gotcha would be to know not to write class”foo” without remembering to also render {{ foo }} else you’ll end up with 2x class attributes.

Just saw the live stream of last week and I like this approach, however I wanted to slide in here and ask a question:
While we can use pure twig with that approach to handle some prepending, appending, replacing, I wonder is it possible to forward attributes somehow?

<{{ attributes.element|default('div') }}
    class=“{{ attributes.class }} foo bar”
    should-override=“{{ attributes.get(‘should-override)|default(‘default-val’) }}
    {{ attributes }}
>
  <twig:SomeOtherComponent an-other-attribute="{{ attributes.something }}" />
</{{ attributes.element|default('div') }}

So in this case, {{ attributes }} will not only already render something-attribute to the main tag, is it actually still available to pass it down to some other element if it maintains a rendered flag?

So maybe this is not the correct approach what I am trying here, but wouldn't it be benefitial to have also an option to not render specific attributes and leave them for later usage, even if {{ attributes }} is rendered already?

[kbond]

@CMH-Benny rendering flag aside, is #1405 similar to what you are thinking?

[CMH-Benny]

@CMH-Benny rendering flag aside, is #1405 similar to what you are thinking?

@kbond Interesting PR for sure, but it's about passing down attributes of a component down to a specific block, which is also interesting to have and a great addition for sure!

I am very interested in symfony-ux lately and try to theory-craft how I could use it in our project and come up with different ideas that raise such kind of questions, so they mostly are theoretical as of now, but before I start to actually implement that, I want to be sure I know what I can do and if I can do what I need.

So this passing down of attributes that are put on a component down to another element was one of those questions and i think it fits here quite well, because append,prepend,merge,replace and preserve is kinda another option for attributes.

I am coming from a different thought, I wondered if it would be possible to create some Interfaces for my Components like "Collabsible" and have a base component template that checks for this interface and in that case adds additional attributes in twig. So every component that implements this Interface and uses the base template as a start point would get this "for free" - But I am not sure if extending/inheriting a twig component template form a base one is even possible, especially because of that attributes handling we have. Don't want to hijack this PR for that, but also not sure where to put that question and since it's related, maybe you can point me to the correct place? Or tell me that I am too crazy 😂

[kbond]

Closing in favour of #1442