Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] Consume a PSR-14 dispatcher for dispatching events#45967
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
3fcc3d5 toc3eedbfComparec3eedbf toa7794bbComparesrc/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
a7794bb to5e04bf4Comparesrc/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| "require": { | ||
| "php":">=7.2.5", | ||
| "psr/log":"^1|^2|^3", | ||
| "psr/event-dispatcher":"^1", |
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 is technically only required if using the queue parts of the library, I think. If it's purely a sync message bus it's not required. That suggests it may be better as a suggests/dev.
PSR packages are small enough that it doesn't really matter, but it's worth mentioning as event-dispatcher wasn't in a first-level require. I'm comfortable either way.
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.
It was, because we could have Symfony's EventDispatcher contracts v1 installed which extended the PSR-14 interface conditionally (= only if it's present), mainly because of PHP 7.1 compatibility (PSR-14 requires PHP 7.2).
I have pushed a new version that should work even in that combination, which allowed me to remove the dependency again.
5e04bf4 toa1213b0Comparea1213b0 tof1169bbCompareUh oh!
There was an error while loading.Please reload this page.
fabpot commentedApr 8, 2022
That's a new feature, and as such, it should target 6.1 (we merged the same for Mailer as a new feature as well). |
f1169bb to78603d0Comparederrabus commentedApr 8, 2022
All right. That makes it a lot easier. 😅 |
78603d0 tof500c7aComparenicolas-grekas commentedApr 11, 2022
Thank you@derrabus. |
Uh oh!
There was an error while loading.Please reload this page.
As@Crell said in#45963, requiring the event dispatcher passed to those classes to be an implementation of Symfony's contract was too restrictive. We can easily consume a PSR-14 dispatcher here.
This PR obviously only fixes the low hanging fruit. The commands coming with the messenger component register event subscribers on the dispatcher, in a way that we need the actual EventDispatcher component for.