Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring#24671
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
| } | ||
| if (isset($this->definedTypes[$type])) { | ||
| returnnewTypedReference($this->types[$type],$type); |
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.
shouldn't we have a deprecation warning triggered here, to tell people to register the type for the new system rather than relying on old types (as you disable the loading in strict mode)
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.
They will get an exception saying that. Not sure there is anything else needed.
nicolas-grekasOct 24, 2017 • 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.
@stof I re-enabled by-explicit-autowiring-type in strict mode, that should address your concern, isn't it?
stof commentedOct 23, 2017
wouldn't this cause issues to use a parameter for that ? Wouldn't any bundle be free to overwrite it ? |
nicolas-grekas commentedOct 23, 2017
that's true, but is this really an issue? |
stof commentedOct 23, 2017
Well, that's precisely the reason why our defaults are file-level. |
nicolas-grekas commentedOct 23, 2017
Well ok. But we always advertised autowiring as a bad idea for bundles before 3.3. I don't think this would be an issue. |
nicolas-grekas commentedOct 23, 2017
I think this is important as we already got several reports by ppl being biten by autoregistration. |
ogizanagi left a comment
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.
Exactly what I needed today 👍
1648936 toa4a0ae2Compare…t from legacy autowiring (nicolas-grekas)This PR was merged into the 3.4 branch.Discussion----------[DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -To preserve BC, autowiring still wires things in hybrid 2.8/3.3 modes.But 2.8 mode is really a foot gun.I propose to add a new parameter in SF3.4, to opt-out of this 2.8 mode, and enable this strict mode for all new projects.WDYT?(seesymfony/recipes#221 for corresponding change on Flex recipe)Commits-------a4a0ae2 [DI] Handle container.autowiring.strict_mode to opt-out from legacy autowiring
Uh oh!
There was an error while loading.Please reload this page.
To preserve BC, autowiring still wires things in hybrid 2.8/3.3 modes.
But 2.8 mode is really a foot gun.
I propose to add a new parameter in SF3.4, to opt-out of this 2.8 mode, and enable this strict mode for all new projects.
WDYT?
(seesymfony/recipes#221 for corresponding change on Flex recipe)