8000 [FrameworkBundle] Add new "routing.annotated_controller" service tag · Issue #21282 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Add new "routing.annotated_controller" service tag #21282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
nicolas-grekas opened this issue Jan 13, 2017 · 10 comments
Closed

[FrameworkBundle] Add new "routing.annotated_controller" service tag #21282

nicolas-grekas opened this issue Jan 13, 2017 · 10 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Milestone
3.3

Comments

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jan 13, 2017 •
edited
Loading
Q A
Feature request? yes
RFC? yes
Symfony version 3.3

When one defines a controller as a service, it would be nice to be able to wire it with routing in a snap of a tag.
eg

services:
    Controller\SomeController:
        tags:
            - name: routing.annotated_controller
              prefix: /foo #optional, maybe the other route params as other tag attributes if worth it?

Using this tag, the routes would be specified as annotations on the related class+methods (which means moving AnnotatedRouteControllerLoader to FrameworkBundle.

This would allow bundles to take control of the wiring of their routing configuration. This means that just by enabling a bundle, its routing config would be done. Of course, each bundle could provide config to change the prefix, disable/enable routes conditionally, etc.

I also think that this would mean one less file to look at for new comers (I mean routing.yml.)

If someone would like to give it a try, please do :)
@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Jan 13, 2017
@nicolas-grekas nicolas-grekas added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Jan 13, 2017
@linaori
Copy link
Contributor
linaori commented Jan 13, 2017

You have my vote for sure! See #12915

@HeahDude
Copy link
Contributor

Although the approach in #12915 was refused, this sounds more reasonable today now that Tobion's comment is resolved and regarding the direction symfony is taking for controllers and bundless apps 👍.

@ro0NL
Copy link
Contributor
ro0NL commented Jan 14, 2017

I'd prefer keeping the trigger and app specific route config in routing.yml (prefix, enabled, etc.)

But the concept of annotated controllers as a service sounds good 👍

@sstok
Copy link
Contributor
sstok commented Jan 14, 2017 •
edited
Loading

I prefer to keep routes in the routing itself, using tags feels to magical and less conventional.
Unless you register them as services and then "import" those service definitions.

Secondly, using Annotations will couple you to the Framework (again) where registering Controllers are service is to prevent that.

To automatically register routes to a pool (for manual importing) you can have a look at https://github.com/rollerworks/RouteAutowiringBundle (and it would be great if this could be moved to the Symfony framework itself, and improved where possible 👍 ).
As a bonus you can use routing files as normal, only the importing is different.

@linaori
Copy link
Contributor
linaori commented Jan 14, 2017

Unless you register them as services and then "import" those service definitions.

This would be nothing more than aggregating the services and then reading the annotations during cache warmup. Instead of having to guess the service id for the class based on annotation, this is probably injected during container compilation.

@dunglas
Copy link
Member
dunglas commented Jan 14, 2017

Secondly, using Annotations will couple you to the Framework (again) where registering Controllers are service is to prevent that.

There is several use cases for controller as a service. One can be decoupling actions form the framework. Another unrelated usage is too enable autowiring on controllers, so you can inject services without defining them in a config file.

The suggestion of @nicolas-grekas makes sense in the 2nd context. The goal is to make the framework easy to use for newcomers, not to decouple from the framework. A more experimented developper can easily switch to framework-agnostic actions later.

I like the idea.

@chalasr
Copy link
Member
chalasr commented Jan 23, 2017 •
edited
Loading

This may fit quite well with what is proposed in #21289 I guess:

App\Controller\:
    autowire: true
    psr4: ../Controller/ # equals the "resource" routing attribute
    tags:
        - { name: routing.annotated_controller }

@nicolas-grekas nicolas-grekas mentioned this issue Mar 6, 2017
Merged
3 tasks
@GuilhemN
Copy link
Contributor
GuilhemN commented Mar 8, 2017 •
edited
Loading

For those who didn't see it, my message on Slack about this:

So to summarize, basically the goal is to pass from

# app/services.yml
App\Controller\:
    autowire: true
    resource: ../Controller/
# app/routing.yml
app:
    type: annotation
    resource: ../../Controller

to

App\Controller\:
   autowire: true
   resource: ../Controller/
   tags:
       - { name: routing.annotated_controller }

, right?

This is a valid use case imo even if the di configuration of controllers should be done by default soon in the standard edition (it reduces the configuration to the routing.yml). So it just depends whether we think this gain is worth it (we win 1 line and it is centralized in a file).

For configuring third party bundles automatically, I think it depends whether flex will support routing.

So, for me, the gain is not big enough (compared to the complexity to implement it) when we're talking about using this in an app. It could be for third party bundles but I think we should first wait and try using flex and then see if it's still needed.

@linaori
Copy link
Contributor
linaori commented Mar 8, 2017

As long as the routes from 3rd party bundles are opt-in and not opt-out *

@nicolas-grekas
Copy link
Member Author

I spend the day talking and thinking about this one.
By leveraging the class <> service-id convention and with the new ContainerControllerResolver, things already work by default now (no need for specifying @Route(service="acme_hello.controller.my_controller")). @iltar is going to give it a try and confirm.
(When the convention is not in use, just fallback to usual wiring, nothing specific.)

The envisioned benefit (being able to wire routes from DI) will require a lot of code, for a discussed benefit: allowing bundle to inject routes may not be a good idea.

The target DX-ness requires only one line in routing.xml also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Milestone
3.3
Development

No branches or pull requests
8 participants
@dunglas @nicolas-grekas @sstok @ro0NL @linaori @GuilhemN @chalasr @HeahDude
0