-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add note about not using autowire with ServiceSubscriber #20961
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
base: 7.2
Are you sure you want to change the base?
Conversation
Without autoconfiguration | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ |
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.
not sure we need a headline here, WDYT of
.. note::
If you are not using autoconfiguration you need to require either ``Psr\Container\ContainerInterface`` or ``Symfony\Contracts\Service\ServiceProviderInterface`` as a ``__construct`` argument or as a method call like ``setContainer(Psr\Container\ContainerInterface $container)``.
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.
Yes I think you are right. I started like this and changed for no good reason. will update
I'm sorry I don't understand what this is trying to solve. |
sure thing @nicolas-grekas . In a bundle we were using the ServiceSubscriberInterface. We couldn't find how to apply the setContainer manually. Because there was a |
I guess you're trying to document the logic in https://github.com/symfony/symfony/blob/dff2ff8bae203d46a44d3fd1f8d7adfa457c8d67/src/Symfony/Component/DependencyInjection/Compiler/ResolveServiceSubscribersPass.php#L32-L34, right? (we should add ServiceCollectionInterface there BTW) Then this relates to autowiring, not autoconfiguration: if not autowiring, those references that point to ContainerInterface/ServiceProviderInterface should be defined explictly. An example in the doc would be nice indeed. |
yes my bad I thought autowire but wrote autoconfigure. You are right indeed. With the PR linked I was trying to solve a bit more than the documentation. Because if you'd like to map several ContainerInterface (never had the case so this is why I won't push more) then you'd be stuck with this auto-binding. If I add examples like $services->set('.sensiolabs_gotenberg.abstract_builder', AbstractBuilder::class)
->abstract()
->call('setContainer', [service(ContainerInterface::class)])
->tag('container.service_subscriber')
; then we should keep it as headline instead of note. Or I am wrong ? |
Without autowire | ||
~~~~~~~~~~~~~~~~ | ||
|
||
If you are not using autowire you need to require either ``Psr\Container\ContainerInterface`` or ``Symfony\Contracts\Service\ServiceProviderInterface`` as a ``__construct`` argument or as a method call like ``setContainer(Psr\Container\ContainerInterface $container)``. |
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.
If you are not using autowire you need to require either ``Psr\Container\ContainerInterface`` or ``Symfony\Contracts\Service\ServiceProviderInterface`` as a ``__construct`` argument or as a method call like ``setContainer(Psr\Container\ContainerInterface $container)``. | |
If you are not using autowiring, you need to reference the special ``Psr\Container\ContainerInterface`` service where the locator should be inject (typically as a constructor argument.) |
|
||
If you are not using autowire you need to require either ``Psr\Container\ContainerInterface`` or ``Symfony\Contracts\Service\ServiceProviderInterface`` as a ``__construct`` argument or as a method call like ``setContainer(Psr\Container\ContainerInterface $container)``. | ||
|
||
.. code-block:: diff |
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.
I guess a .. configuration-block::
would be preferable, since this diff applies to nothing currently?
@@ -140,6 +140,18 @@ count and iterate over the services of the locator:: | |||
The :class:`Symfony\\Contracts\\Service\\ServiceCollectionInterface` was | |||
introduced in Symfony 7.1. | |||
|
|||
Without autowire |
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.
maybe this?
Without autowire | |
Manually wiring a locator |
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.
or a .. note::
indeed as suggested by Oskar
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.
is it possible to add code block in a note ?
What happens if you don't do anything? No explicit service I mean? |
@nicolas-grekas : if you don't add the |
can you try declaring it with no arguments? |
👍 it works indeed. |
OK, so basically this note can be reduced to the case were the injection point is not the constructor + no need to tell about which service to wire, binding does the job |
will update accordingly |
We still need to document the valid targets for the explicit wiring. A common mistake I saw in support request is inject the main |
Resolve #20959