Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedMay 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
nicolas-grekas commentedMay 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I guess you're trying to document the logic inhttps://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: ifnot 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 usingautowire you need torequire either``Psr\Container\ContainerInterface``or ``Symfony\Contracts\Service\ServiceProviderInterface`` as a ``__construct`` argument oras amethod call like ``setContainer(Psr\Container\ContainerInterface $container)``. | |
If you are not usingautowiring, you need toreference the special``Psr\Container\ContainerInterface``service where the locator should be inject (typicallyas aconstructor 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 |
…ctionInterface when declaring a service subscriber (nicolas-grekas)This PR was merged into the 7.2 branch.Discussion----------[DependencyInjection] Fix missing binding for ServiceCollectionInterface when declaring a service subscriber| Q | A| ------------- | ---| Branch? | 7.2| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues | -| License | MITLooks like we forgot this in#53163I figured it out when reviewingsymfony/symfony-docs#20961Commits-------6174d09 [DependencyInjection] Fix missing binding for ServiceCollectionInterface when declaring a service subscriber
Resolve#20959