[PoC] Resolving dependencies automatically

[bizley]

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌
Tests pass?	✔️ (should)
Fixed issues	-

Based on the brief discussion at Slack (https://yii.slack.com/archives/C6H833X42/p1606392783295800) here is my PoC to allow DI container to instantiate class-typed dependencies of an object automatically.

This allows to keep configuration of objects with strings only (class names) and removes the requirement of preconfiguring Container for dependencies (but it's still possible).

I have used mergeDependencies() to resolve dependency with provided argument because in resolveDependencies() it's already too late (original constructor dependency info is lost) so it obviously requires some refactoring (new method maybe, called before mergeDependencies()).

Let me know what you think.

[samdark]

@rob006, @SamMousa what do you think?

[SamMousa]

I don't think the implementation is correct:

1. Usage of a dependency from $b depends on how that dependency is passed in $a.
2. Dependencies from $b are never resolved via Instance::ensure().

Note that whenever a dependency is present in $b we should not resolve an instance from $a at all.

I think merging and resolving should be separate steps for clarity. Since mergeDependencies is actually not related to dependencies I'd rename this mergeNumericalArray, since that is what it does.

I'd add a new function that resolves them and add that to the call sites of mergeDependencies.

[bizley]

Yes, this is exactly what I wrote... This should be moved to a separate method, it's here just as PoC.

[SamMousa]

Fair enough on the clean up, but I think the implementation is also wrong ;-) as in dependencies from $b will be ignored if they are also in $a and defined as instances of Instance..

[bizley]

Hmm, could you give me an example?

[SamMousa]

All untested code:

$existent = new Existent();

// This will throw an exception because it tries to resolve the instance.
assert(mergeDependencies([Instance::of(NonExistent::class)], [$existent]) === [$existent]); 

// This will fail
assert(mergeDependencies([Instance::of(AnotherExistent::class)], [$existent]) === [$existent]); 

// This will pass
assert(mergeDependencies([new AnotherExistent], [$existent]) === [$existent]); 

// This will fail
assert(mergeDependencies([new AnotherExistent], [Instance::of(Existent::class)]) instanceof Existent); 
// This will pass
assert(mergeDependencies([new AnotherExistent], [Instance::of(Existent::class)]) instanceof Instance); 

[bizley]

I have analyzed these examples and maybe when tested in separation from the rest of the class it's like you said, when tested as a whole it's not the case anymore. mergeDependencies() is private. It is used only for a single purpose - to merge the constructor dependencies ($a) with the ones provided ($b). $a will contain object of Instance only when the constructor argument is of class-type (or interface-type). The logic dictates that the corresponding provided dependency must be an instance of that class (or must implement that interface).
I have tried to come with some new method only for the new feature but that would basically repeat what mergeDependencies() does so I think renaming it would be better.
I could not find a case where dependencies are ignored so far, but since this is an important part of framework it should be tested throughout so please help me with that.

[SamMousa]

What if I provide a dependency as Instance::of?

[bizley]

It works as long as object fulfils the requirements of constructor dependency. I'll add example.

[SamMousa]

Ah, so it gets resolved outside the call to merge... This warrants some more research

[bizley]

After deeper analysis I don't think it is something worth to add to the framework, there's simply not enough advantage in instant instantiating vs direct container dependency setting.

If anyone still needs this I've prepared separate package https://github.com/bizley/yii2-deep-instantiate

Thank you @SamMousa for reviewing.