symfony / ux

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

[LiveComponent] AsLiveComponent attribute inheritance #2236

Open jannes-io opened 4 weeks ago

jannes-io commented 4 weeks ago

It would be great if AsLiveComponent could be extended to pre-fill certain values when working on Symfony bundles built on the shoulders of symfony/ux.

Real world use-case: I currently provide a DataTable driven by LiveComponent in one of my bundles. The bundle, and users of the bundle are currently required to use the AsLiveComponent attribute, like so:

#[AsLiveComponent('ItemTable', '@MyBundle/components/table/table.html.twig', csrf: false)]
class ItemTable extends DataTable
{
    // ...
}

This request would be so that "MyBundle" could provide an attribute, #[AsTable()], which would both fill params, and enforce certain values that are expected to be set that way by default. This would enable us to change the underlying default template location, routes, attributes,... without causing a backwards compatibility break for users of the bundle.

For example:

#[\Attribute(\Attribute::TARGET_CLASS)]
class AsTable extends AsLiveComponent
{
    public function __construct(
        ?string $name = null,
        ?string $template = '@MyBundle/components/table/table.html.twig',
    ) {
        parent::__construct($name, $template, null, true, 'attributes', false, 'ux_data_table', 'post', UrlGeneratorInterface::ABSOLUTE_PATH);
    }
}

#[AsTable('ItemTable')]
class ItemTable extends DataTable
{
    // ...
}

Work required: Currently AsLiveComponent is marked final, so that would have to go. Other than that, I have no clue what the implications could be for subscribers, dependency injection,... I assume these aren't built to deal with attribute class inheritance?

smnandre commented 4 weeks ago

    parent::__construct($name, $template, null, true, 'attributes', false, 'ux_data_table', 'post', UrlGeneratorInterface::ABSOLUTE_PATH);    

I assume this is just for illustration purposes, as forcing csrf: false doesn’t seem like a great idea 😅 (nor does forcing the route --since different users may have varying security/firewall constraints).

On a more serious note, this is something doable for Symfony >= 7.1 , so it might be easier to wait for the 3.x version of LiveComponent.

What do you think?

jannes-io commented 4 weeks ago

@smnandre

forcing csrf: false doesn’t seem like a great idea

Let’s just say demonstrative purposes 😅. It’s beside the point but we’ve had multiple reports from different users of csrf throwing errors and causing 400s, so we decided to disable it on non-mutative components and accept the csrf risk rather than cause issues for real users.

This feature would allow us to re-enable it on a later date for everyone without having to update every dependant.

nor does forcing the route --since different users may have varying security/firewall constraints).

Our bundle is more of a platform than just a bundle, we provide security.yaml from our own flex repo, we provide a custom kernel, our own routing,...

it might be easier to wait for the 3.x version

What would be the timeline for this? And without this feature in Symfony, is it something bundles could replicate (maybe even in 6.4)?

smnandre commented 3 weeks ago

What would be the timeline for this?

I'm hoping early 2025.. On my side, I have a lot of features i'd like to work on, and many "bug" reports to deal with... not always easy to find quality time to dev.. so i won't make any promise here :)

And without this feature in Symfony, is it something bundles could replicate (maybe even in 6.4)?

Nope, there is no point to replicate something already available in Symfony 7.1

I guess one possibility would be to allow it... for users using Symfony 7.1 (but we won't document it then, as we don't handle multiple versions at a time for now).

So let's say there is not technical issue here.

The next steps would be:

There is an ongoing work to extract metadata from these attributes, so the next step will probably be to split AsTwigComponent and AsLiveComponent... and then we could do it i suppose.

Or ... we need to work on a method to register components without the AsLiveComponent attribute on the class... but that raises a lot of other questions.

🤷‍♂️