-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Automatic registration of FQCN services #18268
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
Conversation
To me it looks very close to the issue solved by the autowiring... What is the issue you are trying to solve? (at least I think that a big part of the code could be shared) |
46dfd0f
to
fb1e27e
Compare
fb1e27e
to
88ccab4
Compare
|
||
class AmbiguousService | ||
{ | ||
static public function throwException($class, $services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the goal of this class ?
as @Nicofuma said, this is very close to what does autowiring and makes the container even bigger... so I'd say 👎 as is. |
What can be nice instead is an utility class returning the list of services of a given type. (This class can be used internally by the autowiring system and in external bundles needing such features). |
|
@backbone87 it is indeed attractive. To prevent any confusion, why not introducing a new method returning a list of all services of a given type: $container->getOfType(RegistryInterface::class); // the instance of the service of the given type, throw an exception if there is several services of this type
$container->getAllOfType(RegistryInterface::class); // an array of services having the given type |
@dunglas And why introduce a new method (compare it with http://php-di.org/)? What is use case of the public method |
@Ener-Getick @Nicofuma Autowiring is only a special application of FQCN services. Symfony supports this feature for autowired services, but you can't make a reference to service by its type manually. That's inconsistent behavior. |
A new method because it doesn't do the same thing: get retrieves a service by its name, getOfType retrieves a service by one of its types (SOLID code). getAllOfType can be handy to retrieve all your entity managers, or all your elastica finders for instance, or all your API Platform data providers. I can think to a lot of usages. |
@dunglas The service name can be string or FQCN. The string is alias for FQCN and vice versa. services:
Twig_Environment:
class: Twig_Environment
twig:
alias: Twig_Environment What is the correct call, Mentioned reasons for the method |
It would also be nice to make |
The DI container is not a service locator and should not be |
@nicolas-grekas what do you think about the utility class I introduced in #18300 (i changed my mind and decided to create a new class instead of introducing this kind of methods in the container, see #18300 (comment), it's only possible to use it at compilation not at runtime) ? |
@nicolas-grekas Exactly! This PR is introducing automatic registration of service aliases. It has nothing to do with a service locator. |
I created the https://github.com/symfonette/class-named-services library. |
…-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Optional class for named services | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Continues #20264: - makes the id-to-class mapping context-free (no `class_exist` check), - deals with ChildDefinition (which should not have this rule applied on them), - deprecates FactoryReturnTypePass as discussed on slack and reported in this comment: #19191 (comment) From @hason: > I prefer class named services (in applications) because naming things are too hard: ``` yaml services: Vendor\Namespace\Class: class: Vendor\Namespace\Class autowire: true ``` > This PR solves redundant parameter for class: ``` yaml services: Vendor\Namespace\Class: autowire: true ``` > Inspirations: https://laravel.com/docs/5.3/container, #18268, http://php-di.org/, Commits ------- a18c4b6 [DI] Add tests for class named services 71b17c7 [DI] Optional class for named services
I thinks this is solved by #21133, isn't it? |
@TomasVotruba yes, actually it's been working for a while already #21133 just made it easier. There is this however that's still missing and might be interesting: # routing.yml
index:
path: /
defaults:
_controller: 'AppBundle\Controller\MyController:index' |
Closing then, thanks @hason |
All services can be accessed by FQCN, the same concept as for form types: