Skip to content

Conversation

orkhanahmadov
Copy link
Contributor

@orkhanahmadov orkhanahmadov commented Apr 23, 2024

Should solve the issue #50783 (comment)

On Laravel 11 a new method EventServiceProvider@configureEmailVerification() was introduced.

https://github.com/laravel/framework/blob/11.x/src/Illuminate/Foundation/Support/Providers/EventServiceProvider.php#L207-L213.

It works fine when having only a single event service provider, the event gets triggered and the listener works fine. But the problem is when the application has more than one event service provider the listener defined here gets executed multiple times - if the application is using 10 event service providers it will be executed 10 times. It is common to have multiple event providers, especially in DDD applications.

Unfortunately, that new method was implemented without considering applications with more than one default event service provider. It seems too late to change it without causing a breaking change.
Instead, this PR aims to allow developers to turn off the listener on custom event service providers.

An alternative to this would be overwriting configureEmailVerification() on the custom event service provider and leaving it empty, which seems not that nice to me.

protected function configureEmailVerification() {}

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@orkhanahmadov orkhanahmadov changed the title Update EventServiceProvider.php [11.x] Allow turning off listener on Registered event Apr 23, 2024
@orkhanahmadov orkhanahmadov marked this pull request as ready for review April 23, 2024 14:14
@orkhanahmadov orkhanahmadov changed the title [11.x] Allow turning off listener on Registered event [11.x] Allow disabling listener on Registered event Apr 23, 2024
@taylorotwell
Copy link
Member

I think your alternative of just overriding the method is basically equal to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants