[make:listener] use FQCN for kernel events, fix adding unnecessary #[AsEventListener] arg
#1687
Conversation
#[AsEventListener] arg
| @@ -7,7 +7,7 @@ | |||
|
|
|||
| final class BarListener | |||
| 10000 { | |||
| #[AsEventListener(event: RequestEvent::class)] | |||
| #[AsEventListener] | |||
| public function onRequestEvent(RequestEvent $event): void | |||
There was a problem hiding this comment.
Feature creep: can these be invokable?
There was a problem hiding this comment.
Yep!
My only hesitation on this: I sometimes have multiple related listeners in the same file (think security or request/response). I don't know this is a good enough reason to not have invokable listeners though.
There was a problem hiding this comment.
Ideas for better method names:
whenKernelRequest()handleKernelRequest()
or descriptive when possible
- `beforeRequest()
afterException()afterResponse()beforeCommand()afterCommand()beforeLogin()(AUTHENTICATION_SUCCESS)
I quite like the descriptive version
There was a problem hiding this comment.
Do you think this could be generated dynamically? Or some kind of common event map?
There was a problem hiding this comment.
descriptive is better.whenKernelRequest does not tell me at what point of the workflow is executed
There was a problem hiding this comment.
I'm thinking we should go with __invoke() - this would match make:message
There was a problem hiding this comment.
I'm thinking we should go with __invoke() - this would match make:message
I'm not convinced by __invoke here or even in controllers: I don't want a whole new class for a 2nd listener. Message handlers, like commands, really never have a multi-method use-case
you think this could be generated dynamically? Or some kind of common event map?
event map
There was a problem hiding this comment.
Here's my argument for __invoke() (this applies to make:controller also):
- Saves you from thinking of two names: class & method
- Since these makers don't allow adding additional methods, you have to manually add them if desired - renaming
__invoke()to something more descriptive at the same time seems easy.
That being said, I don't feel super strongly. It's easy enough to rename the generated method to invoke and that's what I usually do.
…[AsEventListener]` arg
5ad4063 to
cd1d908
Compare
eventargument to#[AsEventListener]isn't required when it's a FQCN