-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[EventDispatcher] fix empty prio #35260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
symfony/src/Symfony/Component/EventDispatcher/EventDispatcherInterface.php Lines 65 to 67 in 20bf17f
should we update the contract then? Or just the wrongful calling sides? |
@ro0NL thx, good point, pushed changes |
This looks suspicious to me. It looks like a BC break. |
@@ -94,7 +94,7 @@ public function getListeners(string $eventName = null) | |||
public function getListenerPriority(string $eventName, $listener) | |||
{ | |||
if (empty($this->listeners[$eventName])) { | |||
return null; | |||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong to me. The interface states this:
Returns null if the event or the listener does not exist.
Making this change here violates the contract defined by the interface.
@@ -114,7 +114,7 @@ public function getListenerPriority(string $eventName, $listener) | |||
} | |||
} | |||
|
|||
return null; | |||
return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same above
@@ -62,7 +62,7 @@ public function getListeners(string $eventName = null); | |||
* | |||
* @param callable $listener The listener | |||
* | |||
* @return int|null The event listener priority | |||
* @return int The event listener priority, defaults to 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing this is a BC break
returns
0
instead ofnull
as it is typ hinted on other places anyway, and leads to exceptions sometimes.