-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Adding a parameter that allows you to globally set an autowiring map #18040
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
In my opinion it's wrong to have bundles depend or use parameters that are not defined from within the bundle, this is exactly where the configuration comes into play. This is not a parameter but a configuration option for the DIC. The benefit of configuration is that you can enforce the format, whereas this allows users to get all kind of weird errors if the format isn't correct. I would prefer to see something more like: framework:
autowiring-mapping:
Psr\Log\LoggerInterface: my_logger |
I agree with @iltar. Internally, such a config option could still just populate this parameter. But users should not have to rely on this implementation detail. |
The downside with the config option is that it will not be available for people using the standalone component. But, is this feature really necessary? Developers needing such tricks can already override the definition of the service and add the Btw, a new release of MonologBundle has been done and this fix isn't necessary anymore this specific case. |
@dunglas Is there any "easy enough" way to override a service to do this - using If there is an easy way, then maybe we can do that. Otherwise, I can follow the advice of the other guys and move it to config (I don't know why I didn't do that in the first place). |
After talking with @wouterj, there is a way (almost) to do this without needing new config. To continue with the services:
logger.for.autowiring:
alias: logger
autowiring_types:
- 'Psr\Log\LoggerInterface'
What do you think? It actually doesn't work now because |
I much prefer this way. |
that looks good, but what would happen with the following config? # foo.yml
services:
logger.for.autowiring:
alias: logger
autowiring_types:
- 'Psr\Log\LoggerInterface'
# bar.yml
services:
another.logger.for.autowiring:
alias: logger
autowiring_types:
- 'Some\Implemenation\Of\LoggerInterface' |
@iltar with the current implementation, if you typehint |
The thing with the alias is exactly the same as what I originally proposed here #17783 |
@maryo I see that now - it's a clever solution, so I think it didn't "sink in" when you first mentioned it - sorry about that :) |
Actually, this is a non-trivial solution to implement. The problem is that an "alias" service isn't represented as a full Definition object - it's just an To support I was hoping the alias idea would be a nice "shortcut" so that we wouldn't need to add much/any code. But it's not. I recommend we use @iltar's original proposal: #18040 (comment) |
Nevermind, I figured it out. It requires no coding, and is pretty horrible-looking. But, it works (right now, today) and would require not coding. For example, if you assume still that services:
logger_for_autowiring:
class: Psr\Logger\LoggerInterface
public: false
autowiring_types:
- Psr\Log\LoggerInterface
tags:
- { name: auto_alias, format: 'logger' } Yep, that does it - taking advantage of a little-known DI tag. Should we just document this? |
The funny thing is, I actually arrived on that page during the hack-day and I thought it wasn't what you were looking for :( I think it would be an excellent idea to document the auto_alias option and refer to it in the auto-wire documentation. Imo would be best documented as "what to do when you want to autowire but symfony has no support for this case yet". |
@weaverryan: What do others think? Maybe this workarround is acceptable because as Dunglas wrote "it's better to encourage developers to open PRs upstream to add autowiring_types than introducing such settings". |
I think for now, we should stick with this solution. If we find that we're using this a lot (and not just because there is some "bug" upstream that needs to be fixed), we can re-visit adding a clearer solution. But I feel pretty good that there's a workaround, and it's only a few lines of code (though a bit ugly) |
I would like this feature as well. It's very counter intuitive and hard to understa
8000
nd. 5 lines is way too much for simple |
I've added this feature to my bundle, so it's easy to use for everyone without deeper DI+yaml configuration knowledge: |
…icolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Deprecate autowiring-types in favor of aliases | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #21351, #19970, ~~#18040~~, ~~#17783~~ | License | MIT | Doc PR | symfony/symfony-docs#7445 https://github.com/symfony/symfony/pull/21494/files?w=1 This PR deprecates autowiring-types and replaces them by plain aliases. ping @dunglas @weaverryan Eg instead of ```xml <service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false"> <autowiring-type>Doctrine\Common\Annotations\Reader</autowiring-type> </service> ``` just do: ```xml <service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false" /> <service id="Doctrine\Common\Annotations\Reader" alias="annotations.reader" public="false" /> ``` Commits ------- b11d391 [DI] Deprecate autowiring-types in favor of aliases
Hi guys!
This allows end-users to override an autowiring type - e.g.
Using parameters for this was by far the easiest implementation, but not the most user friendly - it's not my favorite to have random magic keys like this do things. There is no precedent (that I'm aware of) for using parameters in this special way. However, why not? If we wanted to deprecate or change it in the future, we can do that easily where we use the parameter.
Cheers!