[RFC] A different way of adding annotations support?

[javiereguiluz]

Description
On Twitter (see https://twitter.com/symfonydocs/status/1161220549785391104) there was a discussion about why we recommend to run composer require annotations in the Symfony routing docs.

This is what we recommend today:

$ composer require annotations

Using version ^5.4 for sensio/framework-extra-bundle
Package operations: 8 installs, 0 updates, 0 removals
  - Installing doctrine/lexer (1.1.0): Loading from cache
  - Installing doctrine/annotations (v1.7.0): Loading from cache
  - Installing doctrine/reflection (v1.0.0): Loading from cache
  - Installing doctrine/event-manager (v1.0.0): Loading from cache
  - Installing doctrine/collections (v1.6.2): Loading from cache
  - Installing doctrine/cache (v1.8.0): Loading from cache
  - Installing doctrine/persistence (1.1.1): Loading from cache
  - Installing sensio/framework-extra-bundle (v5.4.1): Loading from cache

This is what happens with another command:

$ composer require doctrine/annotations

Using version ^1.7 for doctrine/annotations
Package operations: 2 installs, 0 updates, 0 removals
  - Installing doctrine/lexer (1.1.0): Loading from cache
  - Installing doctrine/annotations (v1.7.0): Loading from cache

Both solutions work ... and the second solution looks better because it installs less packages ... but there's an important drawback of the second solution: none of the SensioFrameworkExtraBundle annotations will be installed, such as the ParamConverter ones.

---

There are related discussions like #25361 that proposes to add annotations to core.

In this issue I'm proposing another solution:

1. Create a symfony/annotations package
2. Which uses doctrine/annotations internally of course (we're not going to reinvent an annotation library)
3. And which includes all the annotations from SensioFrameworkExtraBundle.

This way:

• We can get rid of SensioFrameworkExtraBundle
• Annotations in Symfony are only available for people who wants them
• We don't install more packages than needed

What do you think?

[lyrixx]

Big 👍 for moving (almost) everything from SensioFrameworkExtraBundle about annotation to the core

[linaori]

@javiereguiluz this would work very well in combination with [RFC] extended controller/route configuration

[Saracevas]

Using annotations is such an integral part to how Symfony is best experienced today that I think it makes perfect sense to do this. There is an argument for it to be added to the core, but because they're not currently necessary for Symfony to work, I don't think that we should.

[ro0NL]

i would avoid symfony/annotations (it really looks like a annot library :P), in favor of putting them into the component/bridge/bunde they belong to (like Route in Routing, and similar in Serializer).

[linaori]

I agree with @ro0NL on this one. In my scenario the Routing component would provide this feature, so Security bundle/component would provide an annotation for the controller, twig bundle could provide the annotation, etc. This would all be optional, could be a bridge, could be in the bundle directly.

[javiereguiluz]

@ro0NL @linaori let's see if we are talking about the same thing here.

The Routing component already includes the @Route annotation ... but it doesn't work ... because you need an annotation library.

If you run composer require annotations ... you get the annotations library, several annotations (param converters), and some Doctrine packages that you don't need.

If we had a symfony/annotations package, you'd run composer require annotations and you'd get a bunch of annotations made by Symfony and the Doctrine annotation library needed to make them work. Nothing else.

So, if I understood you well, you want to include only the annotation classes but not the annotation library on each component ... so, annotations wouldn't work if you don't install something. It's hard for marketing: Symfony finally includes all annotations in its core ... but ... they don't work ... you have to install something else. 😭

[linaori]

@javiereguiluz my goal is eventually the same as yours, no need to depend on the sensio framework extra bundle. In my scenario I do still want the features but might not want to use annotations to use them, hence I think it works well in combination.

I'm personally in favor of annotations, hence I'd love to see your solution implemented as well as the end result is definitely better. In my RFC it would make it optional to require the annotations because you could use the same features without annotations for controllers.

[ro0NL]

if you run composer require doctrine/annotations symfony/routing doesnt it work already?

then it's simply matter of pointing the annot alias to doctrine/annot :)

and you'd get a bunch of annotations made by Symfony

you'll get those from requiring the component/brdige/bundle, which would be required in the first place to get things to work. So this follows Symfony's "pick what you need" philosophy IMHO.

[javiereguiluz]

@ro0NL yes ... if we move all annotations to different components, then composer require doctrine/annotations would be enough 👍

[MatTheCat]

It seems to me that is what #25361 propose?

[nicolas-grekas]

Anyone willing to move annotations and the related logic from SensioFrameworkExtraBundle to core?

[linaori]

@nicolas-grekas I've been toying with the idea to store the information of the annotations -or controller config for that matter-, somewhere in the Route definition. Would that be an okay way to go? This would utilize the router cache for the configuration and it can be populated in the request attributes. This would keep behavior the same as with the extra bundle and provide this information anywhere during the request life-cycle. Is this something I should continue working upon?

[nicolas-grekas]

store the information of the annotations -or controller config for that matter-, somewhere in the Route definition

nothing shocking to me, it looks like that's what options are fore, isn't it?

[linaori]

Yes, they could be stored within route options and then populated in the request. My only concern is a possible collisions in naming/storage, which made me look into alternatives. Storing it somewhere else makes it a lot more complicated though.

I'll play around with what I have to see if I can make something out of it, though I believe this can be done together with Javier's suggestion.

[stof]

Route options are available for the routing dumpers, but they are not available at runtime. So they don't look like the solution here.
Thus, most of the SensioFrameworkExtraBundle annotations are not related to the route, but to the controller. Attaching annotations to the route is an issue, because the relation between controller and route is 0-n (you can have multiple routes pointing to the same controller, and you can have controllers without routes because they are used in another way with subrequests).
This makes route options a poor fit for these settings.

@javiereguiluz the main argument you have is that SensioFrameworkExtraBundle installs too many dependencies. I think the easy fix here is to make doctrine/persistence an optional dependency of the bundle, as it is needed only when using the DoctrineParamConverter (at which point doctrine/persistence is already required by the ORM or ODM that you are using). Then, we're back at installing only the bundle and doctrine/annotations.

[nicolas-grekas]

Anyone up for a PR?

[dmaicher]

Anyone up for a PR?

I might be able to help out... but what exactly should be done? 😊 I'm a bit lost with the discussion here 😕

[nicolas-grekas]

I think @stof has the correct hint:

I think the easy fix here is to make doctrine/persistence an optional dependency of the bundle

[dmaicher]

See sensiolabs/SensioFrameworkExtraBundle#640

[MatTheCat]

Do we reopen symfony/symfony-docs#10426 then?

[javiereguiluz]

Closing because this will be "soon" irrelevant because annotations will be native starting from PHP 8. Thanks!