[EventDispatcher] fix BC layer #31258
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #31221 |
| License | MIT |
| Doc PR | - |
Simperfit
approved these changes
Apr 26, 2019
chalasr
reviewed
Apr 26, 2019
| @@ -242,7 +243,8 @@ protected function callListeners(iterable $listeners, string $eventName, $event) | |||
| if ($stoppable && $event->isPropagationStopped()) { | |||
| break; | |||
| } | |||
| $listener($event instanceof Event ? $event : new WrappedEvent($event), $eventName, $this); | |||
| // @deprecated: the ternary operator is part of a BC layer and should be removed in 5.0 | |||
| $listener($event instanceof Event || !$listener instanceof WrappedListener ? $event : new WrappedEvent($event), $eventName, $this); | |||
There was a problem hiding this comment.
$event instanceof Event is always false because of the early return l234
and !$listener instanceof WrappedListener is always true as listeners are optimized at this point (plain \Closure).
Broken test case:
diff --git a/src/Symfony/Component/EventDispatcher/Tests/Debug/TraceableEventDispatcherTest.php b/src/Symfony/Component/EventDispatcher/Tests/Debug/TraceableEventDispatcherTest.php
index 8fd0305b0e..db3176dcd1 100644
--- a/src/Symfony/Component/EventDispatcher/Tests/Debug/TraceableEventDispatcherTest.php
+++ b/src/Symfony/Component/EventDispatcher/Tests/Debug/TraceableEventDispatcherTest.php
@@ -18,6 +18,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Stopwatch\Stopwatch;
+use Symfony\Contracts\EventDispatcher\Event as ContractsEvent;
class Trac
8000
eableEventDispatcherTest extends TestCase
{
@@ -151,6 +152,15 @@ class TraceableEventDispatcherTest extends TestCase
$this->assertArrayHasKey('stub', $listeners[0]);
}
+ public function testDispatchContractsEvent()
+ {
+ $tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());
+ $tdispatcher->addListener('foo', function () {}, 5);
+ $tdispatcher->dispatch(new ContractsEvent(), 'foo');
+ $listeners = $tdispatcher->getCalledListeners();
+ $this->assertArrayHasKey('stub', $listeners[0]);
+ }
+
public function testGetCalledListenersNested()
{
$tdispatcher = null;|
#31254 has been updated and fixes the issue mentioned above (with tests) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.