Skip to content
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

Allow injection of container and current definition instance as factory parameters #345

Merged
merged 8 commits into from Nov 29, 2015

Conversation

@quimcalpe
Copy link
Contributor

quimcalpe commented Nov 13, 2015

This PR supersedes #333 and #269. Actually, It's done on top of #333 to preserve commits.

You can get now the container and current definition injected into factory closures type-hinted like this:

factory(function(ContainerInterface $c, Definition $d) {
    return new Thing($c->get($d->getName()));
});

You can mix also any dependency the container can resolve, parameter order is irrelevant with type-hinting:

factory(function(ContainerInterface $c, Service $s, Definition $d) {
    return new Thing($s, $d->getName(), $c->get("whatever"));
});

This is also backwards compatible, so if you don't type hint the first parameter, an instance of the ContainerInterface will be injected, if you don't type hint the second parameter, an instance of the current Definition will be also injected, although that's not recommended anyway.

factory(function($c, $d) {
    $c; // => container instance
    $d->getName(); // => requested name, without wildcards
    ....
});

Todo:

  • test case for FactoryParameterResolver
  • update change-log.md
jdreesen and others added 4 commits Sep 21, 2015
Current Definition can be injected via type-hinting: DI\Definition\Definition
or as a second parameter if no type-hinting is present.

Docs and tests updated accordingly.
@quimcalpe quimcalpe force-pushed the quimcalpe:factory_closure_injection branch from 7665610 to 8867688 Nov 16, 2015
@quimcalpe quimcalpe force-pushed the quimcalpe:factory_closure_injection branch from 8867688 to c7020df Nov 16, 2015
@mnapoli
Copy link
Member

mnapoli commented Nov 29, 2015

I finally got to dive into this! This is a very good addition thanks, I only had a few changes to make. Could you review #347 if you have time? The changes I did are detailed in the PR.

@mnapoli mnapoli merged commit c17949f into PHP-DI:master Nov 29, 2015
4 checks passed
4 checks passed
Scrutinizer 2 new issues, 2 updated code elements
Details
StyleCI The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 84.79%
Details
mnapoli added a commit that referenced this pull request Nov 29, 2015
Inject dependencies and requested entry name in factory parameters [retake of #345]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.