contributte / event-dispatcher

:boom: Best events support (symfony/event-dispatcher) to Nette Framework (@nette)
https://contributte.org/packages/contributte/event-dispatcher.html
MIT License
29 stars 8 forks source link

Dispatcher is not set when used with Kdyby\Console #15

Closed thorewi closed 5 years ago

thorewi commented 5 years ago

Hi guys,

in Kdyby\Console\DI\ConsoleExtension there is:

$sfDispatcher = $builder->getByType(EventDispatcherInterface::class) ?: 'events.symfonyProxy';
if ($builder->hasDefinition($sfDispatcher) && $builder->getDefinition($sfDispatcher)->getClass() === EventDispatcherInterface::class) {
    $app->addSetup('setDispatcher');
}

and this part

$builder->getDefinition($sfDispatcher)->getClass() === EventDispatcherInterface::class

is not true.

I suggest to mention it in readme in Compatibility part. Or am I doing something wrong?

f3l1x commented 5 years ago

Hi.

I don't get it where is the catch. But we can mention it in docs for sure. Don't hesitate to open a PR.

mabar commented 5 years ago

Nothing to do with that package, as it's a kdyby/console bug. Check should be is_subclass_of(EventDispatcherInterface::class) instead of ===

Anyway, we will change type (equivalent of deprecated class) of service from EventDispatcher to EventDispatcherInterface with next major release as it is a BC break for autowiring.

thorewi commented 5 years ago

I know, but there is a Compatibility section here https://github.com/contributte/event-dispatcher/blob/master/.docs/README.md#compatibility where is adviced to do

services:
    events.symfonyProxy:
        autowired: off`

but it's not enough... you need to do something like

$console = $container->getByType(\Symfony\Component\Console\Application::class);
$console->setDispatcher($container->getByType(\Symfony\Component\EventDispatcher\EventDispatcherInterface::class));
    exit($console->run());

or something like

$containerBuilder = $this->getContainerBuilder();
if ($consoleApplicationName = $containerBuilder->getByType('Symfony\Component\Console\Application')) {
        $consoleApplicationDefinition = $containerBuilder->getDefinition($consoleApplicationName);
        $consoleApplicationDefinition->addSetup('setDispatcher');
}

directly in package... that's why I suggested to mention it at least in the readme...

mabar commented 5 years ago

SymfonyProxy cause a collision, so note about compatibility is correct solution. But interface check should be fixed inside kdyby/console so send them PR please.

I could at least add support to contributte/console.

For now, I would suggest you to learn how to configure services. You could configure services added through extension same way as services added through neon.

console.application:
  setup:
    - setDispatcher(@events.dispatcher)

(names of services are just for example, look for their name in DI panel in Tracy bar or in generated container class)

mabar commented 5 years ago

We will solve that in contributte/console#23. Rest is kdyby part of problem.