sulu / SuluHeadlessBundle

Bundle that provides controllers and services for using Sulu as headless content management system
MIT License
45 stars 25 forks source link

Add Attribute for sulu_headless.content_type_resolver tag #135

Closed TheCadien closed 1 month ago

TheCadien commented 10 months ago
Q A
Bug fix? no
New feature? yes
BC breaks? no
License MIT

What's in this PR?

It should be possible to set the required tag, which is needed to load the ContentTypeResolver, via annotation in the future.

Example

#[AsContentTypeResolver]
class ExampleContenType implements ContentTypeResolverInterface
{

    public static function getContentType(): string
    {
        return 'test_type';
    }

    public function resolve($data, PropertyInterface $property, string $locale, array $attributes = []): ContentView
    {
        return new ContentView(['test' => 'foo']);
    }
}

Why?

In Symfony, the configuration process using attributes has become very common. Why Sulu as a Symfony CMS should adapt here

alexander-schranz commented 10 months ago

Attributes works great for things which are not binded to a Interface (Routes, MessageHandlers, ..) where in the past "empty" Interfaces where used, but for kind of things which are binded to a Interface. Autowiring/Autoconfigure should be based on that norml Interface not an additional attribute should be included.

So better autoconfigure the ContentTypeResolverInterface instead of introduce a Attribute here:

        $container->registerForAutoconfiguration(ContentTypeResolverInterface::class)
            ->addTag('sulu_headless.content_type_resolver');
TheCadien commented 10 months ago

I understand your point, I had also considered it, but if I see it correctly, the interface is not used and queried perse, so with the right tagging I can also create the service that way, so I would already offer the dual way as Symfony also offers with EvenSubdcribern, for example

alexander-schranz commented 10 months ago

You mean EventListener? Yeah that is a none related Interface base implementation. But as said above when there is a required Interface there should be go over Interface + Autoconfigure, EventSubscriber are doing the same they don't have additonal Attribute, they have the required EventSubscriberInterface which is being tagged by that interface not a Attribute.