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

Autowiring and Annotations do not work for DI\object() inside arrays #343

Closed
mopahle opened this issue Nov 5, 2015 · 8 comments
Closed
Labels
Milestone

Comments

@mopahle
Copy link
Contributor

mopahle commented Nov 5, 2015

Example code:

<?php
require 'vendor/autoload.php';

class A {
  public function __construct(B $b) {}
}

class B {
  /**
   * @Inject
   */
  public function test() {
    echo "Autowiring and Annotations work.";
  }
}

$builder=new DI\ContainerBuilder();
$builder->useAnnotations(true);
$builder->addDefinitions([
  'test'=>[DI\object(A::class)]
]);
$container=$builder->build();

try {
  $container->get('test');
}
catch (Exception $e)
{
  echo $e->getMessage();
}

Executing the code above gives the following output:

Error while resolving test[0]. Entry  cannot be resolved: The parameter 'b' of A::__construct has no value defined or guessable
Full definition:
Object (
    class = A
    scope = singleton
    lazy = false
)

changing the definitions to $builder->addDefinitions(['test'=>DI\object(A::class)]); works as expected.

@juliangut
Copy link
Contributor

Is DI\object inside an array a correct usage? I mean you're already using DI\object to create the resolvable

@mopahle
Copy link
Contributor Author

mopahle commented Nov 5, 2015

Having only one item in an array may be not really a usecase. But how would you solve something like this without an array:

$builder->addDefinitions([
  'logappenders'=>[
    DI\object(Logappender1::class),
    DI\object(Logappender2::class),
    DI\object(Logappender3::class),
  ]
])

and maybe with a second defintion:

$builder->addDefinitions([
    'logappenders'=>DI\add([
        DI\object(Logappender4::class),
    ])
])

@mnapoli
Copy link
Member

mnapoli commented Nov 5, 2015

That's weird indeed, thanks for the great bug report. The usage seems valid to me and I think I can see why it's not working… After a very quick look I'm not sure what the best fix would be, I'll try to have a look at it soon.

As a workaround (in the meantime):

[
  'test' => [get(A::class)],
  A::class => object(A::class),
];

@mnapoli mnapoli added the bug label Nov 5, 2015
@tuanmh
Copy link

tuanmh commented Mar 11, 2016

I got the same error here. Any plan to fix it?

@mnapoli
Copy link
Member

mnapoli commented Mar 11, 2016

I know what causes the bug (ArrayDefinition doesn't trigger autowiring on sub-definitions) but from what I remember it's not an easy one to fix and I don't have a lot of time available to work on it at the moment (publishing the last Slim bridge should have been 2 weeks ago for example).

For anyone wanting to have a look at it: ArrayDefinition should me made to implement HasSubDefinition based on its items. If an item implements HasSubDefinition then it should trigger a sub-definition resolution (i.e. the autowiring). Problem: an array definition could have 0, 1 or many sub-definitions because it can have many items (and the interface doesn't allow "many sub-definitions" for now). Also it needs to be done with as little performance impact as possible (though the cache should minimize that).

@tuanmh
Copy link

tuanmh commented Mar 13, 2016

Thanks for your quick response & detail guides on this 👍. I'll see if I can help :)

@mnapoli mnapoli mentioned this issue Mar 15, 2016
@leonbobster
Copy link

leonbobster commented Jul 7, 2016

Just a hack, sorry if it completely against your vision: https://github.com/PHP-DI/PHP-DI/pull/426/files
Any feedback is welcome ;-)

@mnapoli
Copy link
Member

mnapoli commented Jun 4, 2017

Hi everyone, this issue has been indirectly fixed by the replacement of DI\object() with DI\create() and DI\autowire(). The good news too is that since the container can now be compiled (#494) performances will not be affected.

@mnapoli mnapoli closed this as completed Jun 4, 2017
@mnapoli mnapoli added this to the 6.0 milestone Jun 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants