Annotation routing in the core #25103
Comments
|
Just an additional note regarding route names: |
|
As Route annotations have been backported to core, I totally like the idea of deprecating |
It's correct that FB does not support it. But I was thinking through it and IMO we don't need it anymore.
The AnnotatedRouteControllerLoader uses "_controller" key which the routing component didn't know about in the past and was independent of that. But with #23227 and similar PRs this independence got removed. So we could inded move it to the component as well.
I think sometimes |
|
AFAIC, I'm using the |
|
Actually |
|
For method one can use FB Route annotation‘s property „method“. Having 2 or 3 ways to configure the same thing in the same place just causes confusion. I am in favor of deprecating/not porting @method and not adding additional route related annotations (except for ones that are meant to be used within a route annotation itself) |
|
I don't see why using more and more annotations when /**
* @Route("/article/{slug}",
* defaults={"slug":"home"},
* requirements={"slug":"\w+"}
* )
* @Method("GET")
*/Would it be more readable than this: /**
* @Route("/article/{slug}",
* defaults={"slug":"home"},
* requirements={"slug":"\w+"},
* methods={"GET"}
* )
*/The method is a part of the route, and having tons of annotations just says that there might be a different logic for the I think we should encourage people using one single |
…favor of Symfony Core (Tobion) This PR was merged into the 5.1.x-dev branch. Discussion ---------- Deprecate routing annotation of FrameworkExtraBundle in favor of Symfony Core Since symfony/symfony#23044 routing annotation is part of Symfony Core. The only extra features that the routing annotation from SensioFrameworkExtraBundle has are - service config for `@Route` which we don't need anymore - Either you use class-named services, then you don't need the "service" property at all. - Or you don't use class-named services. In this case the better solution to me, is for people to create a alias in the container from the controller class to to service id instead of specifying the service id in the routing. Then you're set as well and that is already common practice with autowiring. - `@Method` does not provide real value as people agree in symfony/symfony#25103 This resolves symfony/symfony#25103 Commits ------- 444683c Deprecate routing annotation of FrameworkExtraBundle in favor of Symfony Core
…s (Tobion) This PR was merged into the 3.4 branch. Discussion ---------- Set controller without __invoke method from invokable class | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | | License | MIT | Doc PR | Fixes one part of #25103 Commits ------- cc68c50 Set controller without __invoke method from invokable class
Currently, we have 2 separate annotation loaders for the routing: one in SensioFrameworkExtraBundle (SFEB) and one in FrameworkBundle (FB) which was added more recently. The component only has an abstract class (used by both).
The FB loader was added more recently (in 3.4), as we encouraged using annotation more and more and people wanting to have support in core. But it is missing a few features:
__invoke(probably added in SFEB after the port to core, and not backported)serviceproperty)@Methodannotations (and its subclasses for@GETand@POST). I'm not sure how common they are. An option could be to deprecate them.Having 2 diverging implementations makes maintenance harder (and enabling SFEB changes the loader being used, so adding a new feature in the FB loader is dangerous as installing SFEB would replace it, and the
annotationFlex alias installs SFEB).Here are my suggestions:
__invokein FB so that both loaders are in sync in 3.4 (I would consider it a bug that the implementation is different here)@Route(service="...")and@Method()The text was updated successfully, but these errors were encountered: