yii2: 2.0.36 breaks Codeception tests

It would break for people who have subclassed the application object…

I need to take a closer look. I hope to fix the issue without making the feature optional. Is it ok?

Yeah, I’m aware of the pros and cons from our discussion regarding Yii3 DI package 😉

Anyway, for Yii2 it is a breaking change which to me means we shouldn’t have applied it and therefore should revert it as soon as possible.

If you want to have your use case in a BC way, why not add support to arrays for Instance::of()? Like this could be done in a subclass even:

So we would get:

return [
    ContentTypeMiddleware::class => [
        '__construct()' => [
            Instance::of(StreamFactory::class),
            MultipleInstance::of([
                'json' => JsonFormatter::class,
                'yaml' => YamlFormatter::class,
            ]),
        ],
    ],
];
/**
 * This class represents a string keyed list of dependency definitions that can be resolved by the DI container...
 */
class MultipleInstance extends Instance ... 

This makes it more explicit, while keeping the usability you mention.

@SamMousa I agree with your statement above: for these advanced cases just relying on a \Closure. In our applications we stick with this approach. I even think that

vendor/yiisoft/yii2/di/Container.php:395
$config = $this->resolveDependencies($config);

is redundant. But it’s outside of this ticket.

First of all: #18127 seems to be bad regardless of this specific issue:

• As @DmLapin noted it changes behavior, so it is a BC break
• It introduces (possible) infinite recursion without checks

Then there is the Yii2 issue of attempting to have everything in the configuration array. Things like the logger, errorhandlers and possibly the container get populated from the application config, but they are not part of the application instance.

The application in the codeception module is recreated for every test, but the container is not. As @DmLapin correctly notes, the module uses the container to ask for an application object given a configuration array. If this array refers to objects that are not yet configured, because they are configured by the application at some later stage this will cause issues.

I see several options to remedy the situation:

1. Revert the change from #18127
2. Refactor the config so DI container config lives outside the application config, which from my point of view will improve the code structure regradless of other solutions
3. Have the Yii2 Codeception module parse the configuration array and treat some keys specially

I think 3 is the worst solution; the whole point of the module is to NOT know how the configuration format works, it is not relevant if we let Yii’s own code figure that out. I believe 2 is a bad thing to ask from a user even if it (subjectively, in my view) improves their code; especially considering Yii2 should not be releasing BC breaking versions.

The option 1 is best in my opinion, since we have only had the feature for a very short while there will not be a lot of applications that are not under active development that use it. Apps under active development that use such a new feature will likely be able to step away from it. Upon further inspection, I think the resolveDependencies call should also be more secure in things like forwarding the $reflection variable, since it makes no sense to use that for nested array dependencies…

In conclusion, I propose reverting #18127 and for these advanced cases just relying on a \Closure for configuration of the DI container. It is cleaner and more readable for all PHP users instead of using some secret array language understood only by Yii developers. The array config should be limited to simple cases in all other cases it imo more sensible to use a (static) closure.