Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] add aService attribute to explicitly choose the injected service id#45573
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
kbond commentedFeb 27, 2022 • 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 think it could be nice to have a class MyService{publicfunction__construct( #[Bind('@=service("App\\Mail\\MailerConfiguration").getMailerMethod()')private$service1, #[Bind('@some_service')]private$service2, #[Service('some_service')]private$service3,// equivalent to above #[Bind('%env(json:file:resolve:AUTH_FILE)%')]private$parameter1, #[Bind('%kernel.project_dir%/config/dir')]private$parameter2, #[Bind('%kernel.project_dir%')]private$parameter3, #[Parameter('kernel.project_dir')]private$parameter4,// equivalent to above ) {}}
|
carsonbot commentedFeb 28, 2022
Hey! I think@ruudk has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
nicolas-grekas left a comment• 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.
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.
The implementation and the feature looks fine to me, but there is still this to resolve:
I think it could be nice to have a Bind attribute that allows any yaml syntax to be used:
I would name this#[Autowire], because this wouldn't work like a binding.
If we go with a parameter that supports this yaml-like syntax, we don't need neither Service nor Parameter so both proposals are exclusive to me.
With#44780, we're going to support pretty much everything that this yaml syntax would provide. Only expressions would be missing, and supporting them is only a matter of sending a PR adding a new#[Expression] attribute.
So we have a choice to make to me, which supporting#[Autowire]+yaml-like XORParameter+Service+Expression.
If we go with the latter, I'm wondering if we shouldn't prefix them with Autowire for clarity and disambiguation: AutowireService, AutowireParameter and AutowireExpression.
Note that we already have#[TaggedIterator] and#[TaggedLocator].
kbond commentedMar 6, 2022
I personally prefer a single I still think service/parameter/expression sub-classes could be useful/more explicit but don't feel very strongly about this. Discussion for the future. |
nicolas-grekas commentedMar 7, 2022
Closing in favor of#45657 |
…te (kbond)This PR was merged into the 6.1 branch.Discussion----------[DependencyInjection] add `Autowire` parameter attribute| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets | n/a| License | MIT| Doc PR | todoReplaces#45573 껬 with a single new `Autowire` attribute:```phpclass MyService{ public function __construct( #[Autowire(service: 'some_service')] private $service1, #[Autowire(expression: 'service("App\\Mail\\MailerConfiguration").getMailerMethod()') private $service2, #[Autowire(value: '%env(json:file:resolve:AUTH_FILE)%')] private $parameter1, #[Autowire(value: '%kernel.project_dir%/config/dir')] private $parameter2, ) {}}```Works with controller arguments as well:```phpclass MyController{ public function someAction( #[Autowire(service: 'some_service')] $service1, #[Autowire(expression: 'service("App\\Mail\\MailerConfiguration").getMailerMethod()') $service2, #[Autowire(value: '%env(json:file:resolve:AUTH_FILE)%')] $parameter1, #[Autowire(value: '%kernel.project_dir%/config/dir')] $parameter2, ): Response {}}```Commits-------d43fe42 [DependencyInjection] add `Autowire` parameter attribute
This is a sister PR to#44780. This removes the need to reach for a separate config file when needing to inject a service with a specific service id. This could be used as aless brittle alternative to the
Targetattribute.TODO