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] Allow extensions to create ServiceLocator as services#21770
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
629a14e to7ccc6aaCompare| foreach ($value->getValues()as$k =>$v) { | ||
| if ($v->getInvalidBehavior() === ContainerInterface::IGNORE_ON_INVALID_REFERENCE && !$this->has((string)$v)) { | ||
| continue; | ||
| } |
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.
good catch
5af7880 toc831e2dComparenicolas-grekas commentedFeb 28, 2017
@stof I updated this PR so that ServiceLocatorArgument doesn't need to be a Definition anymore. I now use a new ServiceClosureArgument type instead. See updated PR description. |
c831e2d toea306fcCompare| returnsprintf("function ()%s { return %s; }",$this->dumpValue($reference,$interpolate)); | ||
| } | ||
| returnsprintf("function ()%s {\n return %s;\n }",$this->dumpValue($reference,$interpolate)); |
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.
not enough values|too much placeholders?
should bereturn sprintf("function (): %s {\n return %s;\n }", $type, $this->dumpValue($reference, $interpolate)); at the end, same above (missing colon+space after parenthesis in case the type exists, and $type argument)
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? indentation is desired in the output
the type is not mandatory, always putting the: would be a parse error
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.
So$type misses a: when available and$type is missing as second argument of sprintf?
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.
indeed, fixed
da4e441 to4cf0b83Comparenicolas-grekas commentedFeb 28, 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.
Tests added, PR ready |
db6375f to54855feCompare
chalasr 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.
👍
dunglas 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.
👍
54855fe to8c671ecCompare8c671ec to490745fComparenicolas-grekas commentedMar 4, 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.
I realized that there is a way to decouple autowiring and typed references. |
a8e0b24 to3c2c599Compare3c2c599 tod905d51Compared905d51 to1d96633Comparefabpot commentedMar 5, 2017
Thank you@nicolas-grekas. |
…ices (nicolas-grekas)This PR was merged into the 3.3-dev branch.Discussion----------[DI] Allow extensions to create ServiceLocator as services| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -https://github.com/symfony/symfony/pull/21770/files?w=1With this PR, DI extensions are able to create "service locator" services.They are easily created as such:```php$container->register('my_service_locator', ServiceLocator::class) ->addArgument(array( 'exposed_id' => new ServiceClosureArgument(new Reference('internal_id')), ))```I already need this in two different PRs to come.Commits-------1d96633 [DI] Allow creating ServiceLocator-based services in extensions
…s" tag to inject services into actions (nicolas-grekas)This PR was merged into the 3.3-dev branch.Discussion----------[FrameworkBundle] Add new "controller.service_arguments" tag to inject services into actions| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | (no test yet)| Fixed tickets | -| License | MIT| Doc PR | -Talking with@simensen and@weaverryan, we wondered if we could leverage the `ArgumentResolver` mechanism to make it inject services on demand, using e.g. autowiring.```phpclass PostController{ public function indexAction(Request $request, PostRepository $postRepository) { // PostRepository comes from the container $postRepository->findAll(); // ... }}```This PR achieves that, using a new "controller.service_arguments" tag. Typically:```yamlservices: AppBundle\Controller\PostController: autowire: true tags: - name: controller.service_arguments```It also supports with explicit wiring (thus doesn't necessarily require autowiring if you don't want to use it):```yamlservices: AppBundle\Controller\PostController: tags: - name: controller.service_arguments action: fooAction argument: logger id: my_logger```~~The attached diff is bigger than strictly required for now, until#21770 is merged.~~Todo:- [x] rebase on top of#21770 when merged- [x] add tests- [x] add cleaning pass to remove empty service locatorsCommits-------9c6e672 [FrameworkBundle] Add new "controller.service_arguments" tag to inject services into actions
TomasVotruba commentedMay 1, 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.
I got feeling that I might use this feature... but do not understand it yet. Could you share some more "before/after" examples please? 2-3 would be great, so I could understand it better. |
nicolas-grekas commentedMay 1, 2017
Can't answer in detail now, but please check#21708 it might help, either the code or the feature. |
…es (HypeMC)This PR was merged into the 5.4 branch.Discussion----------[DependencyInjection] Add section about Service ClosuresDocuments service closures added insymfony/symfony#21770 andsymfony/symfony#41176, and the `service_closure()` PHP-DSL function added insymfony/symfony#42625 .Commits-------f2b2fdb [DI] Add section about Service Closures
Uh oh!
There was an error while loading.Please reload this page.
https://github.com/symfony/symfony/pull/21770/files?w=1
With this PR, DI extensions are able to create "service locator" services.
They are easily created as such:
I already need this in two different PRs to come.