Skip to content

[WIP] Use controller autowiring / DunglasActionBundle #315

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.

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

Closed
wants to merge 7 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Mar 20, 2016

@@ -1,18 +1,13 @@
services:
# First we define some basic services to make these utilities available in
# the entire application
slugger:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the interesting part here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a real application, these services would be used in other services, in console commands, etc. You cannot remove them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree than in a real app large app I would explicitly defined at least the slugger service. However, it's not necessary in this sample RAD app and it shows how the new controller system can be used (for RAD only of course).

Just some hints:

  • those services hey are used in other services here (@markdown is used in @app.twig.app_extension)
  • an advantage of using autowiring is if you change signature of the constructor of the AppExtension or if you rename the markdown service, you don't need to update your config
  • if the service using @markdown is itself autowired the @markdown is automatically injected (@app.twig.app_extension)
  • defining explicitly the service (adding back the @slugger definition for instance) doesn't break anything (the named service will be detected and injected everywhere the autowiring service is defined)
  • by convention, all services created by the autowiring system are accessible using the autowired.the\fqn name (but it's better to name them explicitly if you want to access them). Here there is a @autowired.AppBundle\Utils\Slugger service.

Btw, console commands can be autowired the same way than controller, I'm adding an example right now.

@@ -5,8 +5,8 @@
# the routes in YAML, XML or PHP files.
# See http://symfony.com/doc/current/book/routing.html
app:
resource: '@AppBundle/Controller/'
type: annotation
resource: '@AppBundle/Action/'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, keeping the controllers in Controller is better, even if you rely on a special loader aware of the way they are registered as services.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I've opened an alternative PR for that. But I think I'll do it even in the default setup of the bundle. It will be more familiar for Symfony developers (and it seems to be a general concern).

@javiereguiluz
Copy link
Member

I'm closing this as "won't merge" because the initial idea of enabling DunglasActionBundle by default on the Symfony Standard edition was abandoned. Symfony Demo must be "Symfony Standard compliant", so we can't make this change by default in the Symfony Demo. Thanks.

@dunglas
Copy link
Member Author

dunglas commented Apr 21, 2016

@javiereguiluz AFAIK this is not rejected yet (symfony/symfony-standard#960).
But I agree that it will not be for 3.1. (I'll propose an integration path soon).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants