[DependencyInjection] resolve parameters in inlined factories on compile #13455
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -1342,11 +1346,13 @@ private function dumpValue($value, $interpolate = true) | |||
| } elseif ($value instanceof Variable) { | |||
| return '$'.$value; | |||
| } elseif ($value instanceof Reference) { | |||
| if (null !== $this->referenceVariables && isset($this->referenceVariables[$id = (string) $value])) { | |||
| $id = substr($this->dumpValue((string) $value), 1, -1); | |||
There was a problem hiding this comment.
Why ? References cannot use parameters
There was a problem hiding this comment.
Ah, thanks for the hint. I reverted this.
Before this commit, parameters in service configuration would not have been resolved if the new factory syntax (introduced with Symfony 2.6) were used.
3db6416 to
6526dfb
Compare
|
I've investigated this issue further and came up with #13519 which fixes inconsistencies in the "old" way of defining factories (PR for 2.3). When my PR is merged, I will create another on for the new way. |
|
@xabbuh Can you elaborate a bit more about factory methods. As far as I know, the old way of defining factories and also the new way do not support using a parameter for the method. |
|
@fabpot Indeed, only the factory class could be configured via parameters. |
|
closing in favor of #13519 and its successor |
fabpot
added a commit
that referenced
this pull request
Mar 14, 2015
…es (fabpot) This PR was merged into the 2.3 branch. Discussion ---------- [DependencyInjection] fixed service resolution for factories | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #13455 | License | MIT | Doc PR | n/a In the service container, factories can be defined with a class/method pair or a service/method pair. The class or service value can be a container parameter, but it was not supported everywhere, this PR fixes that. Note that the method can never be a container parameter as this is supported nowhere in the current code, so this has not been changed. Another PR will fix the 2.6 way of configuring a factory. Commits ------- f86ad95 [DependencyInjection] fixed service resolution for factories
fabpot
added a commit
that referenced
this pull request
Mar 17, 2015
…factories (xabbuh) This PR was merged into the 2.6 branch. Discussion ---------- [DependencyInjection] resolve class parameters in service factories | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This is based on the parameter resolving for factories from #13519 and makes the necessary changes for the new factory syntax introduced in Symfony 2.6. @fabian In #13519, you also updated the `PhpDumper` to make use of `dumpValue()`. Should I do the same here (or in this case reopen #13455)? Commits ------- 8bda37c resolve class parameters in service factories
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this commit, parameters in service configuration would not have
been resolved if the new factory syntax (introduced with Symfony 2.6)
were used.