Skip to content

Conversation

@Fan2Shrek
Copy link
Contributor

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #61218
License MIT

This is an alternative implementation to support union types, related to issue #61218 and PR #61222.
It duplicates kernel.event_listener tags.

I'm not sure how to add tests for this — could you advise?

@carsonbot carsonbot added this to the 7.4 milestone Jul 27, 2025
@Fan2Shrek Fan2Shrek force-pushed the fix/as-event-listener-multiple-events branch from 4f33247 to a69f5dd Compare July 27, 2025 20:55
@OskarStark OskarStark changed the title Add support for union types on AsEventListener Add support for union types on #[AsEventListener] Jul 28, 2025
@carsonbot carsonbot changed the title Add support for union types on #[AsEventListener] [FrameworkBundle] Add support for union types on #[AsEventListener] Jul 28, 2025
@derrabus
Copy link
Member

I'm not sure how to add tests for this — could you advise?

There should be tests for the AsEventListener feature already. Have a look at them.

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests. 🙃

@Fan2Shrek
Copy link
Contributor Author

Hi @derrabus, thanks for the reply!

The only tests I found are in RegisterListenerPassTest. Is that the right place to add mine?

@Fan2Shrek Fan2Shrek force-pushed the fix/as-event-listener-multiple-events branch from a69f5dd to b872df4 Compare September 24, 2025 12:58
@Fan2Shrek Fan2Shrek force-pushed the fix/as-event-listener-multiple-events branch 3 times, most recently from 277aa0d to 05c0d78 Compare October 1, 2025 14:50
@fabpot fabpot force-pushed the fix/as-event-listener-multiple-events branch from 05c0d78 to 5d7b14f Compare October 5, 2025 07:10
@fabpot
Copy link
Member

fabpot commented Oct 5, 2025

Thank you @Fan2Shrek.

@fabpot fabpot merged commit 40d3c4b into symfony:7.4 Oct 5, 2025
5 of 12 checks passed
This was referenced Oct 27, 2025
nicolas-grekas added a commit that referenced this pull request Oct 29, 2025
…#[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]`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#[AsEventListener] does not handle union types

5 participants