Configurator: add service instances to be injected to container

[VasekPurchart]

When developing extensions I've encountered this problem several times: how to add an instance of a service, which you need to use before the container is created (and the same instance must available for extensions in DIC later.)

There are workarounds for this problem, you can for example define the service with a "dummy" and then "as soon as possible" add the correct instance, such as this example with RobotLoader.

$configurator = new Configurator();
$robotLoader = $configurator->createRobotLoader();

// register extensions

$container = $configurator->createContainer();
$container->addService('robotLoader', $robotLoader);

This usually works, but as most workaround has several problems - if something, that requires a RobotLoader instance, is called before the $robotLoader instance is registered, new (probably misconfigured) service instance is used. This problem would probably occur most of the times due to extensions, which generate code for the Container::initialize method, because this would be called between these two actions and AFAIK there is no way to solve this.

This situation with RobotLoader is only an example, but I think this is a general issue, that's why I am proposing this Configurator::addServices possibility. The way I see it - the usecase is similar to Configurator::addParameters (hence the similar name). These are parameters that we need to use even before the Container is created, this is the same for services. The only difference is that parameters are then statically generated to container, which is IMHO not suitable for service instances. So instances are added after the creation of the whole Container, but before the initialize method.

If there are some services, that require such a dependency e.g. RobotLodaer, the trick with defining the "dummy" should still work like this:

services:
  robotLoader:
    class: Nette\Loaders\RobotLoader

$configurator = new Configurator();
$robotLoader = $configurator->createRobotLoader();
$configurator->addServices(array(
  'robotLoader' => $robotLoader,
));

$container = $configurator->createContainer();

Since the first code which can work with the Container is in the Container::initialize() method, this should become really consistent.

What do you think about this? Do you see any potential problems? Do you have another solution to the problem I outlined above? Right now I am in an deadlock with my extensions because of this issue :(

cc @fprochazka @Vrtak-CZ

[fprochazka]

I like it and I'm :+1: on this.

---

I think you have a typo there

$configurator->addServices(array(
  'robotLoader' => $robotLoader,
));

[VasekPurchart]

Yes, thx, fixed (didn't write it in editor :) )

[matej21]

This problem is partially solved by nette/di@a7a34306 (it doesn't allow to create a new instance of robot loader, but throws the exception)

But I'm not sure I like your solution. What about simplier and more generic solution like new event (e.g. onBeforeInitialize)?

[fprochazka]

@matej21 such event would be on Configurator? Dynamic services in combination with the new event would solve it pretty nicely.

[VasekPurchart]

Dynamic services is only a proper way of writing down the "dummy" from my example (but thx, didn't know about that).

Yes, an event would solve this. I chose this explicit way exacly because the similar usecase with the parameters (so that it is easily usable and understandable). I usually don't like Nette events used in such a way myself, but I have no particular problem with it.

Before I change the PR I would like to have a clear go-for-it, for example from @dg ? :)

[dg]

What about to add option that disables autoinvoking of initialize() ?

[VasekPurchart]

@dg from the extensions point of view that would require the user to do further manual operations and could introduce some unexpected behavior for other situations I think.

My intention is for the extension to provide a static method, which does everything intentally, only needs the instance(s) of requited services. In the RL example this would look like this:

$configurator = new Configurator();
$robotLoader = $configurator->createRobotLoader();
FooExtension::setupBar($configurator, $robotLoader);

$configurator->onCompile[] = function (Configurator $configurator, Compiler $compiler) {
  $compiler->addExtension('foo', new FooExtension());
};

$container = $configurator->createContainer();

So most of the intenal requirements would be still hidden from the user, if manual calling of the initialize method would be needed multiple extensions might have contradicting requirements/duplicate handling of this etc.

Both my PR and @matej21 proposition with events could work the way I wrote.

[fprochazka]

@dg I don't think that is a good idea, because users might disable it and extensions that generate code to initialize will stop working - or worse, you won't know they don't work.

[VasekPurchart]

@dg do you have any thoughts on the two proposed solutions (explicit support, as described in OP, or new event), eventually which one do you prefer?

[VasekPurchart]

@dg ping, please do you have an opinion as to which one of the two proposed solutions do you like better?

[dg]

@VasekPurchart Can you improve the test, there is not covered the main feature, that addServices is called before initialize(). You can for example add option run: yes for new service.

[VasekPurchart]

@dg thanks for the suggestion, I've added an check for the case that addServices is called before initialize and also for service names overwriting.

[dg]

:+1:

[VasekPurchart]

Thanks! :)

[dg]

@VasekPurchart off topic: What is the purpose to add services to DI that are not specified in the configuration (even as dynamic)?

[VasekPurchart]

@dg no good reason that I know of (I don't use it for anything), but since this is supported in the Container I wanted to have it in the test case (as the "easiest" use case opposed to replacing defined service).

[dg]

OK :-)