Invokable entity listeners will definitely be in DoctrineBundle 1.12, so I'm all for merging this PR for consistency 👍

doctrine/DoctrineBundle#989 has been merged - what's the status here?

I guess a review / approval would help 😄

With the bundle supporting this soon, I'm 👎 here: this adds overhead on the critical path of the dispatcher ( a call to SplObjectHash::offsetGet())

The idea was to support it in both kinds of listeners for consistency. Entity listeners are tracked in DoctrineBundle while this PR covers global Doctrine listeners.

I'm talking with @fancyweb on Slack, getting better insight here :)

I don't think this is really a good idea, as using __invoke make things more cryptic. A correctly named method is better. I wouldn't like to open a class and spend more time to discover what it listens too. Yes, local conventions can help but global ones mean broader standardization. Also, having two ways to do something means we're going to have to explain it to ppl. This reasoning applies to doctrine/DoctrineBundle#989 too :)

On the performance side also, please consider my argument: SplObjectHash::offsetGet on the critical path is not free. Using a simple+get_class could remove this objection.

But that's only my first reaction, take it as a maybe hint or ignore it :)
I won't block on this if others think this is still a desired improvement.

I just changed the implementation to not use SplObjectStorage anymore.

get_class cannot be used because the same listener class can technically be used by 2 different events (one with the "__invoke" method and the other with the "eventName" method) (⚠️ but don't do it).

But let's allow __invoke! It will be coherent with the DoctrineBundle + coherent with the Symfony event dispatcher component.

__invoke will only be a fallback, you don't have to use it. We are not trying to force a new way to do things.

I wouldn't like to open a class and spend more time to discover what it listens too.

It depends your code structure. If your listener is in a Doctrine\Listener\LoadClassMetadata you know what you are listening to.

I detected a fail in my implementation and had to rework it. The $methods array have now a depth of 2 to store the methods by event and by hash.

small rebase needed

@nicolas-grekas ✅

Thank you @fancyweb.