Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Adding support to bind scalar values to controller arguments#26658
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
weaverryan commentedMar 23, 2018
Failure is unrelated - unrelated segfault on a different component. |
fabpot commentedMar 25, 2018
Thank you@weaverryan. |
…uments (weaverryan)This PR was merged into the 4.1-dev branch.Discussion----------Adding support to bind scalar values to controller arguments| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25960| License | MIT| Doc PR |symfony/symfony-docs#9477Hi guys!This fixes (I think) the last rough edge with autowiring & error messages. 100% credit to@nicolas-grekas for this implementation - he has generously allowed me to steal his code in return for writing the test. I did test this on a real project.Cheers!Commits-------2c7198c Adding support to bind scalar values to controller arguments
…s (weaverryan)This PR was squashed before being merged into the master branch (closes#9477).Discussion----------Removing warning about bind + controller + scalar valuesDocs forsymfony/symfony#26658 :)Commits-------af00dbc adding version note about scalar bind args4312518 Removing warning about bind + controller + scalar values
| $args[$p->name] =newReference('value.'.$container->hash($bindingValue)); | ||
| $container->register((string)$args[$p->name],'mixed') | ||
| ->setFactory('current') | ||
| ->addArgument(array($bindingValue)); |
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.
As this service is here to build a scalar, I should we should mark it as non-shared, to avoiding registering the scalar in$this->services
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.
why? what's the issue with sharing a scalar (or array btw?)
looks better shared to me, did I miss something?
| if (!$bindingValueinstanceof Reference) { | ||
| continue; | ||
| $args[$p->name] =newReference('value.'.$container->hash($bindingValue)); |
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.
I would use_value as prefix, to reduce the likeliness of conflicts with an actual service, as the YAML format forbids using the underscore prefix in service ids since 4.0.
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.
fixed inba05588
fabpot commentedMar 27, 2018
Thinking about this one, how to you differentiate between something coming from the routing ( |
javiereguiluz commentedMar 27, 2018
If I remember this correctly, the difference is the priority of the resolvers ... this one that resolves container params has low priority (lower than the routing one, which always wins over this one). |
Uh oh!
There was an error while loading.Please reload this page.
Hi guys!
This fixes (I think) the last rough edge with autowiring & error messages. 100% credit to@nicolas-grekas for this implementation - he has generously allowed me to steal his code in return for writing the test. I did test this on a real project.
Cheers!