[Console] Fix ConsoleEvents::SIGNAL subscriber dispatch #45333
Conversation
GwendolenLynch
commented
Feb 7, 2022
| Q | A |
|---|---|
| Branch? | 5.4 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Tickets | Fix #45332 |
| License | MIT |
|
Thank you for your PR! Can you please add a test hat covers your change? |
|
Added @derrabus. The act of writing the tests showed up some missing bits, so thank you for the nudge. Changed
Please let me know what changes you'd like made. |
GwendolenLynch
force-pushed
the
command-signals
branch
2 times, most recently
from
44c9186
to
0b7cd1d
Compare
|
Thank you @GwendolenLynch. |
| } | ||
| if ($this->signalsToDispatchEvent) { | ||
| $commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : []; | ||
| $dispatchSignals = $this->dispatcher && $this->dispatcher->hasListeners(ConsoleEvents::SIGNAL); |
There was a problem hiding this comment.
$this->dispatcher is typed to be a \Symfony\Contracts\EventDispatcher\EventDispatcherInterface, but hasListeners is not part of that contract.
This leads to our package breaking with the current branch version (5.4.x-dev in our case), since it does not fulfill the \Symfony\Component\EventDispatcher\EventDispatcherInterface
|
@GwendolenLynch @nicolas-grekas I added a comment regarding using |
|
This should be fixed. Up for a PR? |
…y with the contract interface (xabbuh) This PR was merged into the 5.4 branch. Discussion ---------- [Console] fix dispatch signal event check for compatibility with the contract interface | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #45333 (comment) | License | MIT | Doc PR | Commits ------- 6b29591 fix dispatch signal event check for compatibility with the contract interface
