-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[FrameworkBundle] Add support for union types on #[AsEventListener]
#61252
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
[FrameworkBundle] Add support for union types on #[AsEventListener]
#61252
Conversation
4f33247 to
a69f5dd
Compare
#[AsEventListener]
#[AsEventListener]#[AsEventListener]
There should be tests for the |
derrabus
left a comment
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.
Please add tests. 🙃
|
Hi @derrabus, thanks for the reply! The only tests I found are in RegisterListenerPassTest. Is that the right place to add mine? |
a69f5dd to
b872df4
Compare
277aa0d to
05c0d78
Compare
05c0d78 to
5d7b14f
Compare
|
Thank you @Fan2Shrek. |
…#[AsEventListener]` (HypeMC) This PR was merged into the 7.4 branch. Discussion ---------- [EventDispatcher][FrameworkBundle] Rework union types on `#[AsEventListener]` | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT This is a reimplementation of #61252. The previous PR introduced a backward compatibility break. Consider the following listener: ```php final class TestListener { #[AsEventListener(event: RequestEvent::class)] public function onRequestEvent(): void { // ... } } ``` In earlier versions, this worked fine, but now it throws: > AsEventListener attribute requires the first argument of "App\EventListener\TestListener::onRequestEvent()" to be an event object. Interestingly, there *was* a test for this scenario, but since each test method re-defines the `registerAttributeForAutoconfiguration()` closure (which wasn't updated everywhere), the tests still passed. Additionally, the implementation was added to the `FrameworkExtension`, even though similar logic already existed in `RegisterListenersPass::getEventFromTypeDeclaration()`, resulting in a decentralized implementation. This PR reverts the changes in `FrameworkExtension` and re-implements the feature in `RegisterListenersPass`. The tests now reuse the closure from `FrameworkExtension` to make them more robust and consistent with the actual implementation. Commits ------- 8ea7196 [EventDispatcher][FrameworkBundle] Rework union types on `#[AsEventListener]`
This is an alternative implementation to support union types, related to issue #61218 and PR #61222.
It duplicates
kernel.event_listenertags.I'm not sure how to add tests for this — could you advise?