-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[2.8] Make service not shared when prototype scope is set #15091
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
👍 |
Tests should be fixed now as well :) |
👍 |
@@ -33,6 +33,6 @@ | |||
<configurator class="%baz_class%" method="configureStatic1"/> | |||
</service> | |||
<service id="factory_service" class="Bar" factory-method="getInstance" factory-service="foo.baz"/> | |||
<service id="foo_bar" class="%foo_class%" scope="prototype"/> | |||
<service id="foo_bar" class="%foo_class%" shared="false" scope="prototype"/> |
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.
Do we need the additional prototype
attribute?
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.
this is to test the output of the XmlDumper. As this service is a legacy service, it only sets the scope to prototype (which remains the same). The only thing this PR changed was also setting shared to false in these cases
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.
@fabpot btw, do you think that we need to keep the correct |
And shouldn't we add a dedicated test for this to avoid regressions? |
@xabbuh Yes, we need a test to avoid regressions |
👍 with an additional test |
Added a very simple legacy test |
👍 |
1 similar comment
👍 |
Thank you @wouterj. |
…t (WouterJ) This PR was merged into the 2.8 branch. Discussion ---------- [2.8] Make service not shared when prototype scope is set | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Deprecating the scopes was introducing a BC break in Symfony 2.8 when using the prototype scope. As `$shared` wasn't updated if the scope was set to `prototype`, people will always get an error because of [the `CheckDefinitionValidityPass`](https://github.com/symfony/DependencyInjection/blob/04d5d925a987f35854f51b1c468c0ca81e1d61b9/Compiler/CheckDefinitionValidityPass.php#L54-L57). Commits ------- b7030cc Make service not shared when prototype scope is set
Deprecating the scopes was introducing a BC break in Symfony 2.8 when using the prototype scope. As
$shared
wasn't updated if the scope was set toprototype
, people will always get an error because of theCheckDefinitionValidityPass
.