Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy 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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
4f33247 toa69f5ddCompare#[AsEventListener]#[AsEventListener]#[AsEventListener]
There should be tests for the |
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 tob872df4Compare277aa0d to05c0d78Compare05c0d78 to5d7b14fCompareThank you@Fan2Shrek. |
40d3c4b intosymfony:7.4Uh oh!
There was an error while loading.Please reload this page.
…#[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 | MITThis is a reimplementation of#61252.The previous PR introduced a backward compatibility break.Consider the following listener:```phpfinal 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?