[DependencyInjection] Add optional named arguments support #21711
Conversation
|
Thanks @nicolas-grekas for the syntax, I didn't find better so I used it. |
|
Why can't the syntax be the following? arguments:
$locale: '%app_locales%'
$sender: '%app.notifications.email_sender%' |
|
Because we want to catch typos which means throwing an error when there is a named argument that is not used, so we need a syntax saying that the user doesn't want the exception. |
|
@GuilhemN do we throw an exception when an application defines a service and doesn't use it? I don't know why we should throw an exception when defining an argument and not using it. The only exception should be when a required argument is not defined anywhere. |
|
@weaverryan any thought on this discussion since you were directly involved in the current design? |
|
@javiereguiluz not when defining services, but when referencing them you'll get an exception if one doesn't exist and you didn't explicitly ask to ignore errors ( |
|
I can see what we're trying to fix / make more clear: https://github.com/symfony/symfony-demo/pull/483/files#r102006501 Currently, when a service can't be autowired, we have one way of fixing this: add a service definition for that one service, and "complete" the service definition. That's nice, because, the process is always the same: "Your service can't be autowired? Just manually register the service and add the missing argument". This proposal reduces the amount of code needed, but I'm not sure it's as clear. It also introduces this new syntax... and we already have too many (special YAML syntaxes are one of the least-popular things when I teach Symfony). Overall, So, this is a well-thought-out proposal, but I'm -1 on it. If the config is looking a bit ugly (I admit, it's not all that nice for the symfony-demo - here's a before and after the linked PR, without comments https://gist.github.com/weaverryan/7caebf2acfe115e46e3e67c7e3eca8b8), then we should try to think of other ways to improve it. I'm not sure what that is - I'm staring a lot at the config right now, trying to "get used to it" or think of why it looks a bit ugly :). |
|
My first thought was |
|
Another idea, what if we use the services:
Foo:
autowire: { $foo: 'foo', $bar: 'bar' }it could then be used for constructors but also for method calls
Manually overriding services looks ugly to me :) |
|
Closing as I prefer @nicolas-grekas's proposal (making named arguments optional for _instanceof and prototypes), I'll give it a try this weekend. |
This PR was squashed before being merged into the 3.4 branch (closes #22187). Discussion ---------- [DependencyInjection] Support local binding | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | #22167, #23718 | License | MIT | Doc PR | > A great idea came out on Slack about local bindings. > We could allow injecting services based on type hints on a per service/file basis: > ```yml > services: > _defaults: > bind: > BarInterface: '@usual_bar' > > Foo: > bind: > BarInterface: '@alternative_bar' > $quz: 'quzvalue' > ``` > > This way, `@usual_bar` will be injected in any parameter type hinted as `BarInterface` (in a constructor or a method signature), but only for this service/file. > Note that bindings could be unused, giving a better solution than #22152 to #21711. > > As named parameters are usable in arguments, bindings could be usable in arguments too: > ```yml > services: > Foo: > arguments: > BarInterface: '@bar' > ``` ~Named parameters aren't supported yet.~ Edit: > Note that bindings could be unused Current behavior is throwing an exception when a binding is not used at all, in no services of a file if it was inherited from `_defaults` or in no services created from a prototype. It will pass if the bindings are all used in at least one service. Commits ------- 81f2652 [DependencyInjection] Support local binding
This PR was squashed before being merged into the 3.4 branch (closes #22187). Discussion ---------- [DependencyInjection] Support local binding | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | symfony/symfony#22167, #23718 | License | MIT | Doc PR | > A great idea came out on Slack about local bindings. > We could allow injecting services based on type hints on a per service/file basis: > ```yml > services: > _defaults: > bind: > BarInterface: '@usual_bar' > > Foo: > bind: > BarInterface: '@alternative_bar' > $quz: 'quzvalue' > ``` > > This way, `@usual_bar` will be injected in any parameter type hinted as `BarInterface` (in a constructor or a method signature), but only for this service/file. > Note that bindings could be unused, giving a better solution than symfony/symfony#22152 to symfony/symfony#21711. > > As named parameters are usable in arguments, bindings could be usable in arguments too: > ```yml > services: > Foo: > arguments: > BarInterface: '@bar' > ``` ~Named parameters aren't supported yet.~ Edit: > Note that bindings could be unused Current behavior is throwing an exception when a binding is not used at all, in no services of a file if it was inherited from `_defaults` or in no services created from a prototype. It will pass if the bindings are all used in at least one service. Commits ------- 81f2652 [DependencyInjection] Support local binding
This PR was squashed before being merged into the 3.4 branch (closes #22187). Discussion ---------- [DependencyInjection] Support local binding | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | symfony/symfony#22167, #23718 | License | MIT | Doc PR | > A great idea came out on Slack about local bindings. > We could allow injecting services based on type hints on a per service/file basis: > ```yml > services: > _defaults: > bind: > BarInterface: '@usual_bar' > > Foo: > bind: > BarInterface: '@alternative_bar' > $quz: 'quzvalue' > ``` > > This way, `@usual_bar` will be injected in any parameter type hinted as `BarInterface` (in a constructor or a method signature), but only for this service/file. > Note that bindings could be unused, giving a better solution than symfony/symfony#22152 to symfony/symfony#21711. > > As named parameters are usable in arguments, bindings could be usable in arguments too: > ```yml > services: > Foo: > arguments: > BarInterface: '@bar' > ``` ~Named parameters aren't supported yet.~ Edit: > Note that bindings could be unused Current behavior is throwing an exception when a binding is not used at all, in no services of a file if it was inherited from `_defaults` or in no services created from a prototype. It will pass if the bindings are all used in at least one service. Commits ------- 81f2652 [DependencyInjection] Support local binding
Add optional named arguments support:
The goal is to permit registering a bunch of services with a set of arguments that will be used only if necessary.
An example of use case: https://github.com/symfony/symfony-demo/pull/483/files