[Twig] Add prepend to component attributes

[1ed]

Q	A
Bug fix?	no
New feature?	yes
Tickets	#612
License	MIT

As Stimulus controllers' lifecycle methods called in the order as they appear
in the DOM. If the order of these calls matters for our use case we need a way
to add a controller before others. We can of course wrap the whole thing in a div
and put our controller there, but sometimes that is not convenient (like using flexbox) and for live controllers we can not do it actually.

TODO:

• docs

[kbond]

I'm thinking we should rename these two methods (addController/prependController) maybe?

[1ed]

I think it's better with controller too, maybe appendController/prependController? Twig component is not experimental iirc, so should we deprecate add and not just rename it?

[kbond]

There hasn't been a release that includes add yet so we can still rename.

[1ed]

IMHO we should change the type hint to StimulusControllersDto as it only works with that correctly. Or we should create a generic method and decide what to do based on the type. Or perhaps something like this:

    public function append(array|AbstractStimulusDto $attrs): self
    {
        return $this->mergeAttribute($attrs);
    }

    public function prepend(array|AbstractStimulusDto $attrs): self
    {
        return $this->mergeAttribute($attrs, true);
    }

    private function mergeAttribute(array|AbstractStimulusDto $other, bool $prepend = false): self
    {
        $otherAttributes = $other instanceof AbstractStimulusDto ? $other->toArray() : $other;
        $selfAttributes = $this->attributes;

        $attributes = [...$selfAttributes, ...$otherAttributes];
        foreach ($selfAttributes as $name =>  $attr) {
            $attributes[$name] =
                trim(
                    ($prepend ? $otherAttributes[$name] ?? '' : $attr)
                    .' '.
                    ($prepend ? $attr : $otherAttributes[$name] ?? '')
                , ' ');
        }

        return new self($attributes);
    }

[kbond]

I like this. I believe Ryan was concerned with the verbosity of attributes.appendController(stimulus_controller(...

[1ed]

@weaverryan what is your preference? I like append/prepend better too, should we continue with it?

[weaverryan]

I like your proposal to do something like array|AbstractStimulusDto... because, why not?

I'm not sure about append/prepend. Or, I want to try to push back on it a little bit :). The order of the Stimulus controllers should not matter. I think that is a "design problem" with how we currently fetch "Component" instances - and we should fix that a different way - probably solution (C) from here #612 (comment)

So, assuming we fix that, is there really any reason to have a prepend vs append? That's not something you normally care about with element attributes.

[1ed]

I'm not sure about append/prepend. Or, I want to try to push back on it a little bit :). The order of the Stimulus controllers should not matter. I think that is a "design problem" with how we currently fetch "Component" instances - and we should fix that a different way - probably solution (C) from here #612 (comment)

I found this test value_list_observer_tests.ts#L26-L35, which tells me that the order of tokens in the same attribute matters for Stimulus, because changing data-controller="one" to data-controller="two one", it first disconnects one and after that it connects two and after one again, to ensure two connected first and one after that. Of course it's better to not rely on that in our code, but that maybe not always possible.

For example

// foo_controller.ts
export default class extends Controller<HTMLElement> {
    override connect() {
        this.dispatch('connect')
    }
}

// hello_controller.ts
export default class extends Controller<HTMLElement> {
    onFooConnect(e: CustomEvent) {
        alert('foo connected')
    }
}

This will not work

<div data-controller="foo hello" data-action="foo:connect->hello#onFooConnect">

but this will work

<div data-controller="hello foo" data-action="foo:connect->hello#onFooConnect">

This definitely seems like an edge case to me and I can't come up with a good practical example.

[weaverryan]

Hi!

Coming back to things :). Looking back at the original issue - #612 - the use-case is very straightforward: we need a way to reliably get the Component instance from another Stimulus controller. Currently, the live controller will initialize first and then your custom controller. And so, if you're listening to live:connect in your custom Stimulus controller, it's too late!

IMO the proper solution is the ComponentRegistry type of idea from #621. In that case, order doesn't matter (but, nicely, since the live controller is initialized first, your custom controller will be able to get the Component immediately: it'll already be available). And your custom controller code would look like:

import { Controller } from '@hotwired/stimulus';
import { ComponentRegistry } from '@symfony/ux-live-component';

export default class extends Controller {
    async connect() {
        this.component = await ComponentRegistry.get(this.element);
    }
}

WDYT @1ed? Can we close this in favor of the ComponentRegistry idea?

[1ed]

Hi, I hope you enjoyed your holidays. This is not interesting for live controllers only, but for stimulus in general. But we can close this and reopen if it happens to have a good use case.

[weaverryan]

This is not interesting for live controllers only, but for stimulus in general. But we can close this and reopen if it happens to have a good use case

Hmm... indeed I could imagine possibly some use-case. But yes, I'd like to wait. I don't want people to have to make a choice (prepend or append?) unless they absolutely have to :).

Thanks for the convo :)

[norkunas]

It would be have to great this, as it's also useful when having many components that define data-actions, but also need to append the additional actions for some cases, so doing the manual merges in twig components is awful :(

At least for my case would be nice to control which attributes should be merged into a single one, instead of totally overriding, as currently only a class attribute is this special.