Proposal: Put back the Annotations and PHP7 compatibility

[gesof]

I noticed that PHP7 support was dropped a few days ago (and annotations support) in favor of PHP8 attributes. In the long run it is a good idea but it will take us a while for transitioning to PHP8 and therefore testing the new UX components will not benefit of community feedback as much as it would if PHP7 would still be an option.

There is a huge trend of pushing the business logic toward Javascript and client-side programming which is somehow abnormal in my opinion. One of the reasons is the lack of alternatives and the UX component is so promising to fill in the gap. But if it takes 1-2 years to become a standard I'm afraid we'll all loose a big opportunity to keep the web programming sane.

[nicolas-grekas]

This choice was made in order to spend our time on building the actual thing rather than making it work with phpdoc. There are challenges to add support for php7 that are not worth this time.

Said another way, if you want to see this happen, the best and maybe only way would be to send a PR.

Up to giving it a try?

[gesof]

Well, the support for PHP7 was there already last week. Maintaining it may require additional work but the benefits reflect into the present by immediate adoption.

[weaverryan]

Well, the support for PHP7 was there already last week. Maintaining it may require additional work but the benefits reflect into the present by immediate adoption.

The support was there, until we replaced the "wiring guts" of the components. So, in reality, what happened is that we had a system that supported PHP 7, then we totally replaced it with a new system (and decided not to add PHP 8 support then). So, it is kind of a thing that needs to be re-added.

However, I would be very happy if someone added it :). Probably we would need to extract the logic that reads things from the PHP 8 attributes (search the codebase for AsTwigComponent and AsLiveComponent) into a separate service. Then, we could make two "versions" of that service: one that reads attributes and one that reads from annotations. A config option would swap between the 2 services.

Note: if if we did the above, users would still (unless we did more work) need to manually tag their components with twig.component, but that's fairly minor :).

[gesof]

Will Symfony 6 have support for annotations, like routing and mapping ?

[wouterj]

I think existing readers will not be removed, but none of the newer attributes are implemented as annotations.

I think the gist here is: it is doable, but we don't want to invest time in this as we don't see the need. However, this is open source, so we are happy to work together with someone that has a need for php 7 and time to implement it. (Quick note, php 7.4 reaches eom in November this year, personally I favor putting time in upgrading to php 8 instead)

[gesof]

Using PHP8 attributes like this just feel wrong for two reasons:

1. the new "attributes" are meant to add meta information to classes and properties, like configurations, and they should not really be considered "code", in other words they should not replace code like "instanceof(interface)" with "$reflection->getMethods()->getAttributes()".
2. However, because there was a need in the past to decorate class properties, annotations were invented and used all over. Dropping them in a rush to embrace the new attributes when there is a ton of code that uses annotations (doctrine and other bundles) will just delay the transition to SF6 (or maybe only to UX). Actually there will be no transition, it is a new start, a new generation of coders will have to learn the framework and use it because the rest of us will have to maintain older platforms.

[wouterj]

(1) is exactly why annotations and now PHP attributes are invented. While attributes are PHP syntax, it's not code. It's metadata.

I don't really follow (2) if I'm honest. Moving from @Route to #[Route isn't that big of a change. PHP developers have overcome a transition from procedural code to proper classes and from a loosely types language to a strict typed language. It seems developers are pretty great in learning new things :wink: We also don't really rush here: PHP 8 is almost a year old and PHP 7 has almost reached the end of maintenance.

[kbond]

the new "attributes" are meant to add meta information to classes and properties, like configurations, and they should not really be considered "code", in other words they should not replace code like "instanceof(interface)" with "$reflection->getMethods()->getAttributes()".

I agree with @gesof on this point regarding AsTwigComponent/AsLiveComponent. I personally still think they should be interfaces. There are several spots where we are checking if an object is a component using attributes. This feels a bit like an instanceof hack. For LiveProp/LiveAction and the other method hooks, they make perfect sense to me as attributes/annotations.

[nicolas-grekas]

There are several spots where we are checking if an object is a component using attributes

Really? This should be a red flag. Attributes must have purely declarative semantics. Can you link to such a code please?

[kbond]

Anywhere we are using AsTwigComponent/AsLiveComponent::forClass($object). If the object does not have the attribute, an exception is thrown. Here's an example of it being used.

[nicolas-grekas]

Then there is clearly something wrong with the design. Name/template must be carried out with DI tags instead.

[weaverryan]

Hi everyone!

Thanks for the constructive convo - nothing is set in stone, so let's figure it out :).

The line between what should live in an interface and what should live in "external config" (whether that is annotations, attributes, XML, YAML) is fuzzy to me. Every Twig component needs a name & a template. Here are the two arguments in my head:

1) Interfaces: If every component needs these, then add an interface with getName(): string and getTemplate(): string.

2) External Config (e.g. attributes/annotations): A twig component is a normal object. If you decide that you would like this to be used in the twig component system, then similar to validation, Doctrine entities or API Platform "api resources"), you add some extra config that controls how it should be used in that system.

The line between when something should be in an interface and when something should be in "external config" looks blurry to me, but the external config (2) feels more correct here, and is the same that we see in other systems, like API Platform. The one piece of information that we need a "compile time" is the component name so that we can create the lazy service locator.

Is there a clear rule that can guide to the correct decision here?

[nicolas-grekas]

External config should never be done direclty in annotations/attributes, because that creates tight coupling to info that must remain declarative. E.g if you enable the annotation driver, then you opt-in into using these. But if the code hardcodes that these have to be taken into account, there is a significant design issue.

Using DI tag attributes might do it here. At least this looks similar to how eg commands are wired.

[weaverryan]

External config should never be done direclty in annotations/attributes, because that creates tight coupling to info that must remain declarative. E.g if you enable the annotation driver, then you opt-in into using these. But if the code hardcodes that these have to be taken into account, there is a significant design issue.

Would the issue be solved if we also allowed the config to be specified in XML/YAML? I'm trying to see how your statement - "External config should never be done directly in annotations/attributes" is consistent with validation, entity & API Platform config, which all use those.

Using DI tag attributes might do it here. At least this looks similar to how eg commands are wired.

That's true - though only the name (and I think description?) in commands are in the tag, which were motivated by laziness. The command "help" and command arguments are not in the tag (they're also not in annotations/attributes, I admit - but my point is that some command config lives in the DI tag and some command config lives elsewhere).

[nicolas-grekas]

Would the issue be solved if we also allowed the config to be specified in XML/YAML?

Yes, but I'm not suggesting doing that :) DI tags can already be defined using either yaml, XML or annotations.

in commands are in the tag, which were motivated by laziness.

Actually, the tag predates the need for laziness. It's been motivated by software design :)

The question should be: is the name/template in the wiring domain? Or is it a concept that a component must know about? I feel like it's a wiring concern, they should thus be externally defined/or at least definable. Exactly like commands. Makes sense?

[wouterj]

I agree that the line between interfaces and external config is blurry and even more so because we're dealing with all the legacy before attributes (if you look at e.g. Java, attributes are a much more significant concept than annotations have ever been).

What triggered me to suggest removing the interface was the fact that the interface only contained 1 (and later 2) getters. Imho, that's a clear sign of a marker interface and not a "behavioral contract". Imho, marker interfaces are superseded by attributes since PHP 8 and we shouldn't fall back to the legacy way of handling markers for new features.

[kbond]

After a slack discussion with @weaverryan, here's what we propose:

Make the current AsTwigComponent/AsLiveComponent attributes completely optional (opt-in). It's purpose will be for DI auto-configuration and optionally adding configuration to your component (right now just name/template/default-action but we could add more later if required). Down the road, if desired, we could add alternate ways to configure your component: annotations/yaml/xml.

This, I believe, is consistent with how api-platform registers/configures resources.

To manually configure component services, you'd tag the service with twig.component and add the component's name via the key tag attribute (name is reserved). This would be the only tag attribute. Template/Default Action/(future options) are part of the configuration (attribute-only right now).

Thoughts?

[tgalopin]

I like the idea of not introducing annotations but relying on manual declaration using services. It avoids introducing features that'll be removed in a year or two.

If we can make it that it's possible to use autoconfiguration on a given namespace, we could perhaps emulate the way controllers are registered as controller.service_arguments? This would mean having the naming logic based on getDefaultName() for the tagged locator, but with https://github.com/symfony/symfony/pull/40248 this would be very nice on Symfony 5.4/6.0.

[wouterj]

I'm not sure I can understand the reasoning.

What I'm understanding at the moment: The downsides of "reinventing" configuration readers are the need to maintain different loaders, something of a cache system (you don't want to do it at runtime). The downsides of using a DI tag for all this is mostly the need for 1 custom "configuration" service to provide component details based on the component name?

Another important question imho is: how is a user going to override the template? (i.e. when using a third party component, but you want custom styling)

[kbond]

The downsides of using a DI tag for all this is mostly the need for 1 custom "configuration" service to provide component details based on the component name?

To clarify, you're in favour of adding all config to the DI tag via tag attributes? I'm not necessarily opposed it just seems unconventional to me somehow. It's hard to know all the config options that will eventually be added and this is probably a non-issue but what if an option is an array? I don't think tag attributes (at least when defined in xml) can be an array.

Another important question imho is: how is a user going to override the template?

This is a good question and would be solved easily if config was done via DI tag attributes.

[wouterj]

I guess the answer is, is this component a service or a data holder?

For services, it's common to provide configuration via the DI tag (e.g. kernel.event_listener and monolog.processor). For data holders, it's more common to use specialized annotations, yaml, xml (e.g. Doctrine entities, API Platform resources and Serializer).

Needing to use a DI tag (even in the proposed implementation) makes it a service in any case, which is why I favor the simplicity of using the DI tag. Some comments in this issue compare it with Doctrine entities and API Platform resources. This does look like you're thinking about components as data holders. This does question whether there should even be a tag.

I think that's an even more fundamental question to this issue: what type of object is a component?

[kbond]

I guess the answer is, is this component a service or a data holder?

Heh, good question and my answer would be: both. It's a service that, when instantiated, holds data. We're doing something, as far as I'm aware, completely novel.

To clarify, the service is shared: false so it isn't a stateful service.

[kbond]

Unless someone disagrees, I'll create a PR that removes the requirement for AsTwig/LiveComponent and makes components fully configurable via tag attributes.

[klemens-u]

Hi, first of all - thank you very much for your great effort on Symfony UX. I wanted to add a personal view on this: I was bummed when I found out that Twig Components require PHP 8.0. A random reality check:

• Lot's of bigger projects are only now getting ready for PHP 7.4
• Many current LTS versions like Ubuntu 20.04 support only PHP 7.4, the next one is coming only in April, so actually usable only in Fall 2022 (I'm aware of PPAs ;-))
• In my personal case we're in progress of porting a legacy app to symfony (5). No way we can bring that app up to PHP 8.0. Now that's a pity because in our case of a port it would be wise to use the upcoming future standards like Twig Components.

So if possible, PHP7.4 support would be very welcome!

[tgalopin]

As said on this issue, we are definitely open to a PR @klemens-u for annotations support. The best person to add it is you, as you have the need for it :).

[kbond]

With #127 we removed the strict requirement on attributes to wire-up components. They can now be manually configured. The remaining blocker for 7.4 is the property/method attributes (ie LiveProp). Annotations, and all the required code for this would need to be added for these. I agree with @wouterj's comment above. I would add that even if implemented, it would add to the maintenance burden of these libraries.

[klemens-u]

As said on this issue, we are definitely open to a PR @klemens-u for annotations support. The best person to add it is you, as you have the need for it :).

I know, but sorry that is beyond my capabilities right now... Still I tried to contribute something through my feedback.

[wouterj]

Thanks for trying out Symfony UX during the migrating to Symfony though. I hope this will work out nicely :)

Given the comments here, I think it's better to close this issue to indicate the current opinion on this matter. To summarize what is said above:

• Attributes are not required. Since #127 (Symfony UX 2.0), you can wire Twig/Live components using service tags.
• Given the state of PHP 7.4 (end of life), the minimum requirement for the latest Symfony releases (PHP >8.0) and the complexity to integrate Doctrine Annotations, there are no plans to add support for annotations.