-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Support for parameter injection #20738
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
How does this deal with snake_case parameter names? |
There may be some interests and advantages to it, but I would at least add a configuration parameter besides just setting |
@kasparsklavins it doesn't deal with it, just like for request attributes, if your parameter is |
I may not be the most objective person to argue about this, because I don't like much autowiring (it's quite opinionated, I know), but I wonder if this isn't going too far with this 😅 .
Considering this, my own preference clearly is to not deal with parameters injection at all 😄 |
@ogizanagi your concerns are justified and 1) should definitely be avoided. However, I don't think that it's so common to have several arguments/parameters with the same name and in such case (as for all "complex" cases), the user should note use autowiring at all and switch to the classic definition. I guess that conventions will change and that the common usage when using autowiring will become something like: namespace PaymentClient;
class PayPalClient
{
public function __construct($PayPalApiKey) {}
// instead of
// public function __construct($apiKey) {}
} namespace PaymentClient;
class StripeClient
{
public function __construct($stripeApiKey) {}
// instead of
// public function __construct($apiKey) {}
} # app/config/config.yml
parameters:
payPalApiKey: aze
stripeApiKey: rty If (and only if), services are build automatically (like when using ActionBundle) with 0 config, I think it worths it.
Thanks to this set of features, it will be very easy to create (and refactor) services, without having to know about the container (the service container becomes a framework internal and will never be used directly by the user): you just have to create the class and it works, Symfony wires everything for you. And after all, it's just a parameter name, it's not part of the public API of the class, it's an internal. When the application starts to mature or to grow, you can still disable autowiring and rename the parameter without breaking anything. I predict that we'll soon have a command to "write" the definition of an autowired service to transform it in a standard (not autowired) service. It will help too. |
I know the intention of all the recent "magic" functionality is to improve DX and lessen the learning curve to help newcomers. This general goal is great. Don't get me wrong, I love the idea of easing the on-boarding for new developers, but... I don't agree that adding more "magic" and hiding away anything complex will necessarily accomplishes this goal. In fact, I think in some ways, it is detrimental to it.
No! Please, no! Don't remove the need to understand the container! Doing do will result in new developers who are blindly ignorant as to the true flexibility and power that is available to them (thanks to the amazing architecture of this framework). Honestly, in the years I've been using Symfony, one of the most profound moments --- the one that I believe propelled me from an inexperienced Symfony developer to a knowledgeable, skilled one --- was the day that all the basic building blocks Symfony is comprised of finally "clicked." It wasn't until after this experience that I would consider any of my projects with this framework as "good". And it wasn't until after this experience that I truly had fun creating Symfony projects. Trying to shield developers from some of the most significant features of Symfony is not how we should go about attracting new ones, IMHO. These are the same aspects and features that have continued to make Symfony the most compelling choice as a web framework over the last few years. We want to make sure people do have to learn about these things. Until they do, they will only be scratching the serfice of what is possible in Symfony. When I first started using Symfony, sometime around 2011 with the introduction of v2.0 (never used 1.0), the documentation was solid and only getting better. By the time I was using Symfony in a large project in 2012, the documentation was extensive. Now, since then, the documentation has only continued to improve and develop into, what I consider, one of the most extensive, rich, and full-featured reference manuals for any open source framework---one that even bests a large majority of commercial frameworks! This is the most important piece to keep current and improve upon to help newcomers. (IMHO) Performing "magic" actions for the user is always sketchy in my book, regardless of the intent, so I don't like this to begin with, likely skewing my reaction to this PR, but I simply feel like we should work toward better real-life examples that show proper usage of complex components instead of try to make the framework do everything for the user. I don't wan to allow newcomers to ignore important components and concepts to their own workflow determent. As for this PR in general, I'd love to see it written in such a way as to incorporate scalar type hinting (when available) and then fallback to the simple variable name matching currently implemented. It would be great if it knew not to pass a matching parameter/variable if the parameter type doesn't match the expected variable type. Also, I feel very strongly that this should complement the current |
I kinda like it :)) but it should go after #20167 imo. So it only applies to white listed methods. Parameter/variable naming is less an issue then, as it couldnt be autowired otherwise anyway.
Makes sense to me. |
I agree with @theofidry and @ro0NL, this feature should be optional. It should be possible to use service autowiring and parameter autowiring separately. I propose the following config: # app/config/services.yml
parameters:
hello: SymfonyCon
services:
'Foo\Action\Homepage':
autowire: true # Default to constructor autowiring + parameter autowiring
# Alternative
autowire:
methods: [__construct, set*]
parameters: false And indeed, #20167 must be merged first. |
By introducing this new services:
'MyLogger':
autowire:
bind: Psr\Log\LoggerInterface
bind: MyCustomInterface |
I think adding
If you autowire |
@ro0NL, the default value if |
@dunglas about the last suggestion what about: services:
'MyLogger':
autowire:
bindings:
- Psr\Log\LoggerInterface
- MyCustomInterface ? |
…(dunglas, nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DependencyInjection] Add support for named arguments | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | todo This PR introduces named arguments for services definitions. It's especially useful to inject parameters in an autowired service. It is (at least partially) an alternative to #21376 and #20738. Usage: ```yml services: _defaults: { autowire: true } Acme\NewsletterManager: { $apiKey: "%mandrill_api_key%" } # Alternative (traditional) syntax services: newsletter_manager: class: Acme\NewsletterManager arguments: $apiKey: "%mandrill_api_key%" autowire: true ``` ```php use Doctrine\ORM\EntityManager; use Psr\Log\LoggerInterface; namespace Acme; class NewsletterManager { private $logger; private $em; private $apiKey; public function __construct(LoggerInterface $logger, EntityManager $em, $apiKey) { $this->logger = $logger; $this->em = $em; $this->apiKey = $apiKey; } } ``` Commits ------- 8a126c8 [DI] Deprecate string keys in arguments 2ce36a6 [DependencyInjection] Add a new pass to check arguments validity 6e50129 [DependencyInjection] Add support for named arguments
It's basically the same idea than request's attributes injection as controller's parameters: if a service is autowired and a container's parameter match, this parameter is injected. Example: