[Twig] ComponentAttributes::append()/prepend()

[kbond]

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

This is a followup to #1442.

When reviewing #1416, I noticed how using ~ to build attribute values with just render() was a bit annoying. This PR adds the following:

1. Default value to render() (render('class', 'default')) to avoid needing to use render('class') ?? 'default'
2. prepend()/append() to avoid needing to to use class="{{ attributes.render('class') }} some defaults"

Example:

{# templates/components/MyComponent.html.twig #}
<div
  class="{{ attributes.render('class', 'default') }}" {# second parameter is the default value #}
  style="{{ attributes.append('style', 'display:block;') }}" {# second parameter value to append #}
  data-controller="{{ attributes.append('data-controller', 'hello-controller') }}" {# second parameter value to prepend #}
  {{ attributes }} {# be sure to always render the remaining attributes! #}
>
  My Component!
</div>

{# render component #}
{{ component('MyComponent', { style: 'color:red;' }) }}

{# renders as: #}
<div class="default" style="color:red; display:block;" data-controller="hello-controller">
  My Component!
</div>

[kbond]

When working on this I kept getting confused by append/prepend (what's prepended/appended, the real value or the default?).

I think in most cases the order here doesn't matter, or am I wrong? Could we reduce to a single method that just joins a default value with the actual value? If so, what should this method be called?

[smnandre]

prefix ?

[WebMamba]

Haha thanks @kbond! I had the same idea while working on my PR! You are too fast! 😁

[kbond]

I think in most cases the order here doesn't matter, or am I wrong?

The order does indeed matter when using tailwind_merge.

[smnandre]

So .after('alert') and .before('activated') ?

[WebMamba]

I know this already merged but what do you think about the following syntax:

   <div
    class="{{ attributes.render('class', 'default') }}" {# second parameter is the default value #}
    style="{{ attributes.render.before('display:block;') }}" {# second parameter value to append #}
    data-controller="{{ attributes.render.after('hello-controller') }}" {# second parameter value to prepend #}
    {{ attributes }} {# be sure to always render the remaining attributes! #}
>
  My Component!
</div>

[kbond]

So .after('alert') and .before('activated') ?

Yes, that seems more explicit - maybe .renderAfter()/renderBefore()?

I know this already merged but what do you think about the following syntax:

I originally had render return an object where we could chain these types of methods. I think it reads nicer too. We ended up dropping as it removed the possibility to do:

{{ attributes.render('class') ?? 'default' }}

Since render would always return the object ?? would never work.

Seeing this is a super new feature, we could probably get away with sneaking in this obscure BC break?

[WebMamba]

I am voting to sneak a bit here 😈

[kbond]

I am voting to sneak a bit here

Me to, @weaverryan, @smnandre, any objections?

I'm thinking the following:

{# templates/components/MyComponent.html.twig #}
<div
  class="{{ attributes.render('class').default('default') }}"
  style="{{ attributes.render('style').before('display:block;') }}" 
  data-controller="{{ attributes.render('data-controller').after('hello-controller') }}"
  {{ attributes }}
>
  My Component!
</div>

[smnandre]

  class="{{ attributes.render('class').default('default') }}"

 class="{{ attributes.render('class') ?? 'default' }}"

?

[smnandre]

I'd feel easier to get something like renderAfter / renderBefore but as you want

[kbond]

class="{{ attributes.render('class') ?? 'default' }}"
?

This would no longer work (and be the BC break here). render() would always return the new object.

I'd feel easier to get something like renderAfter / renderBefore but as you want

I don't have a strong opinion but yes, renderAfter/renderBefore would be a bit simpler and not create a BC break.

[smnandre]

Hmm... if you make this object countable that'd be helpful :)

[WebMamba]

What do you think if the function renders instead of returning null return a new object called ComponentAttribute? This new ComponentAttribute object is Stringable. So we should not have a BC break. And we can put the function before and after on this class directly. I thinking about that because if I use the function render('class'), I expect something that represents the class attribute and nothing more, but if I understand well your implementation we get a clone of the full ComponentAttributes object.
So with my proposal we can do:

// this work since ComponentAttribute is Stringable
class="{{ attributes.render('class') ?? 'default' }}"

// but also all of this
<div
  class="{{ attributes.render('class').default('default') }}"
  style="{{ attributes.render('style').before('display:block;') }}" 
  data-controller="{{ attributes.render('data-controller').after('hello-controller') }}"
  {{ attributes }}
>
  My Component!
</div>

so the function default, before, and after will be on the ComponentAttribute object

[kbond]

Wouldn't it need to be changed to the following?

- {{ attributes.render('class') ?? 'default' }}
+ {{ attributes.render('class') ?: 'default' }}

Also, and we'd need to double check, but at the time ?? or ?: is evaluated, it would be an object still, wouldn't it?

[WebMamba]

Ha yes sorry, so the function render returns null or the new ComponentAttribute object.

[smnandre]

If you implement countable the ?: would work

[kbond]

so the function render returns null or the new ComponentAttribute object.

I don't want it to return null as that would require an extra check when chaining .before()/after().

If you implement countable the ?: would work

Why would it work for countable but not a stringable object that returns an empty string? I'm not a huge fan of making it countable just for this purpose.

[smnandre]

Oh it was just to give a fun fact :)) I'm 100% ok with your proposal :)

Count is called before empty, i discoverd that after a loooong afternoon of debugging (when Jane Automapper started to use ArrayObject with no public properties in a minor version 😅.... making every |default working in the opposite way than excpected :p )

[smnandre]

After a while i'm starting to thing.... why passing to a method something that can be direct in the HTML ?

<div style="{{ attributes.render('style').before('display:block;') }}" >

is much easier to read as

<div style="display:block; {{ attributes.render('style') }}">

Same for "after"... no ?

[smnandre]

And what could improve DX would be magic getter i think

<div style="display:block; {{ attributes.style }}" class="{{ attributes.class }}">

[kbond]

Same for "after"... no ?

Totally valid and maybe preferred by some. The primary reason I added was to make it prettier when using something like tailwind_merge.

And what could improve DX would be magic getter i think

One problem with using the magic methods to avoid calling ->render() is there's no longer an error for a bad method call (ie a typo in defaults()): {{ attributes.dfaults({class: 'block'}) }} === null

[smnandre]

Ok lets Forget then :)

[weaverryan]

Hi!

A) The 2nd arg of render() (a default value) was motivated by the ~ usage in #1416 - {% set alert = cva('alert ' ~ attributes.render('class'), {. But I have an alternate idea #1416 (comment)

B) The prepend/append idea seems to be entirely wanted / needed JUST for tailwind_merge. In all other situations, something like style="display:block; {{ attributes.style }}" I think is totally fine. For tailwind_merge, the current syntax would require:

class="{{ (attributes.render('class')~' bg-red-500')|tailwind_merge }}"

That is a bit ugly :). Could tailwind_merge perhaps be offered also as a function with unlimited args (note, this is how the underlying php library is meant to be used).

class="{{ tailwind_merge(attributes.render('class'), 'bg-red-500') }}"

Or this?

class="{{ [attributes.render('class'), 'bg-red-500']|tailwind_merge }}"

tailwind_merge already supports receiving an array... and the underlying library supports even those arrays being string with spaces in them - e.g. $tw->merge(['text-red-500 text-sm'], 'text-blue-500');.

[kbond]

I agree, I like the idea of unlimited arguments in both cva and tailwind_merge. It really seems like the most explicit way to prepend/append when compared to:

When working on this I kept getting confused by append/prepend (what's prepended/appended, the real value or the default?).

[kbond]

Closing as there are, I think, better solutions.