EventServiceProvider Bug when upgrading a Laravel 10 to Laravel 11 using the new Application Structure (as a result of Laravel Shift)

[mathiasgrimm]

Laravel Version

11.22.0

PHP Version

8.3.10

Database Driver & Version

No response

Description

In Laravel 10, the EventServiceProvider extends Illuminate\Foundation\Support\Providers\EventServiceProvider which will cause issues if we continue to extend it since there's no information in the upgrade guide that we should stop extending it.
In the Upgrade Guide, there's no reference to event, discovery, or listener. The upgrade guide even has this:

Laravel 11 introduces a new default application structure with fewer default files. Namely, new Laravel applications contain fewer service providers, middleware, and configuration files.

However, we do not recommend that Laravel 10 applications upgrading to Laravel 11 attempt to migrate their application structure, as Laravel 11 has been carefully tuned to also support the Laravel 10 application structure.

Laravel 11 will by default boot the Illuminate\Foundation\Support\Providers\EventServiceProvider in addition to booting your app/Providers/EventServiceProvider.php, if registered in the providers.php, meaning all listeners present in the Listeners folder will be auto-discovered in addition to having them also register manually in App\Provider\EventServiceProvider.

To me, this seems to be a double issue. Laravel 11 default behavior is changing, meaning:

1. Some listeners that were never registered are now registered
2. Events that were registered will be triggered twice.

I've only found it because the project had some specific tests, but it might be hard to spot it otherwise, affecting a lot of people and even not causing problems it could end up using more server resources unnecessarily.

Since the Illuminate\Foundation\Support\Providers\EventServiceProvider continues to use array_unique in the register method, as it did in Laravel 10, when attaching listeners, I assume the intention is to continue to keep them unique.

With that in mind, I have made the following changes. I can also create a PR to provide this fix.
This fix will prevent the Illuminate\Foundation\Support\Providers\EventServiceProvider and only it, from registering an event that's already registered which fixes the first issue without affecting anything. I'm not tackling issue 2 and just wanted to raise this concern.

There seems to be a way to disable this behavior by using a non-documented method :

\Illuminate\Foundation\Support\Providers\EventServiceProvider::disableEventDiscovery();

or using `withEvents(false)``

protected function listenerAlreadyRegistered($event, $listener): bool
    {
        $existingListeners = Event::getRawListeners()[$event] ?? [];

        $listenerNotFullyQualified = str_replace(['@handle', '@__invoke'], '', $listener);

        return in_array($listenerNotFullyQualified, $existingListeners)
            || in_array($listener, $existingListeners);
    }

/**
     * Register the application's event listeners.
     *
     * @return void
     */
    public function register()
    {
        $this->booting(function () {
            $events = $this->getEvents();
            foreach ($events as $event => $listeners) {
                $existingListeners = Event::getRawListeners()[$event] ?? [];

                foreach (array_unique($listeners, SORT_REGULAR) as $listener) {
                    // THIS IS THE CHANGE - It follows the same __CLASS__ pattern found in another part of this class
                    if (get_class($this) == __CLASS__ && $this->listenerAlreadyRegistered($event, $listener)) {
                        continue;
                    }

                    Event::listen($event, $listener);
                }
            }

            foreach ($this->subscribe as $subscriber) {
                Event::subscribe($subscriber);
            }

            foreach ($this->observers as $model => $observers) {
                $model::observe($observers);
            }
        });

        $this->booted(function () {
            $this->configureEmailVerification();
        });
    }

Steps To Reproduce

Upgrade from L10 to L11 using Laravel Shift or try to use the new not-recommended Application Structure.
Considering Laravel Shift is a suggestion shown in the Upgrade Guide I think its worth checking.it's

[mathiasgrimm]

Laravel Shift actually didn't suggest to keep using the EventServerProvider, but by removing it, it stopped registering some Listeners which to register manually could potentially cause them to run in a different order (potentially).

In any case, all listeners in the Listener folder would be registered which is not desired.

What Laravel Shift does is incentivize to use the new application structure which ultimately leads to these two issues, which could also happen when doing it manually.

[crynobone]

Wouldn't the best solution and easiest to document is to add call to disableEventDiscovery() on bootstrap/app.php (based on Laravel 10 structure)?

\Illuminate\Foundation\Support\Providers\EventServiceProvider::disableEventDiscovery();

[mathiasgrimm]

Thanks for taking the time to review it. I appreciate it.

Yep, I think that works.
I've just installed a fresh L10 and migrated manually to L11 without changing the Application Structure and it worked as expected. The issue is more related to Laravel Shift trying to migrate the structure then.

Thanks.

[jasonmccreary]

@mathiasgrimm, feel free to email Shift support if you feel there is something that could be improved with the automation.

Without full detail on your app events, all I can say is that over 20,000 apps have been successfully upgraded with the Laravel 11.x Shift to the new slim structure. So any issue likely relates to the specifics of your app code rather than Laravel 11 or Shift itself.

Again, glad to improve on any automation or warnings. Just email. 👍