Add wildcard support to event discovery

[jared-cannon]

Overview

As someone who uses the "Features/" project structure for my larger applications, a way to automatically scan each new feature's Listeners directory is a nice, albeit simple, QOL improvement. My suggested change would live in the EventServiceProvider->discoverEvents() function, and would look like this:

Code change

/**
 * Discover the events and listeners for the application.
 *
 * @return array
 */
public function discoverEvents(): array
{
    return (new Collection($this->discoverEventsWithin()))
        ->flatMap(function ($directory) {
            // Use glob to expand wildcard patterns into directory paths
            return glob($directory, GLOB_ONLYDIR);
        })
        ->reject(function ($directory) {
            return ! is_dir($directory);
        })
        ->reduce(function ($discovered, $directory) {
            return array_merge_recursive(
                $discovered,
                DiscoverEvents::within($directory, $this->eventDiscoveryBasePath())
            );
        }, []);
}

Result

This would allow a user to add the following line to their withEvents() function:

->withEvents(discover: [
    __DIR__.'/../app/Features/*/Listeners',
])

Addendum

This is my first proposed change for Laravel. After reading the contributing documentation, I wasn't sure whether I needed to open an issue before submitting a PR - I'd appreciate any guidance on this. I'm happy to create a PR with this change along with an update to the documentation describing this functionality.

[Sophist-UK]

Personally I think that Laravel should provide much better support for large scale applications in which large sections need to be structured as modules or features.

In essence it should be possible by either composer.json entries or php calls in service container initialisation to tell Laravel of any directories that have a similar structure to '/app' or indeed to add specific directories for any specific types of file that Laravel searches for automatically (like Events or Configuration files etc.).

Edit: Also to avoid unnecessary scans of these directories on every Laravel invocations - and with e.g. Livewire these can be very frequent very small invocations - every type of automatic search like this should be cached. (I would even go so far as to say that wherever possible a low cost hash of the source should be created and the cache should be refreshed automatically when the hash changes.)

Providing recursion is disallowed, I see no performance problems - scanning /app/Features for subdirectories and then scanning each of those subdirectories for a Listeners subdirectory is NOT a lot of extra overhead, but scanning recursively inside /app/Features for a Listeners subdirectory at any depth can have a very significant performance hit.

I had experience looking at some packages that attempt to add this "module" functionality, and one that implicitly used a recursive directory structure /**/Listeners resulted in a massive set directories being scanned on every php invocation resulting in almost 1s delay to every transaction (which was obviously completely unacceptable).

The important thing for me is that this needs to be a) part of Laravel core, and b) performs well, and c) flexible, and d) easy to code.

[jared-cannon]

Definitely agree with your thoughts on performance. My assumption is that all production environments cache the discovery of event listeners via php artisan event:cache, so my proposed change should not affect performance on production or staging environments.

[shaedrich]

I had experience looking at some packages that attempt to add this "module" functionality, and one that implicitly used a recursive directory structure /**/Listeners resulted in a massive set directories being scanned on every php invocation resulting in almost 1s delay to every transaction (which was obviously completely unacceptable).

When this is needed, that could probably be optimized via caching in non-dev environments 🤔

[Sophist-UK]

Actually caching all discovery should be something that should be done automatically. I would even go so far as to suggest that if possible some low-cost automated means of cache refresh should be implemented wherever possible. For example you could cache the MD5 hash of the composer.json file and refresh the cache whenever that changed.

[shaedrich]

When this is needed, that could probably be optimized via caching in non-dev environments 🤔

This could build off of this PR: #52928 (comment)

[jared-cannon]

Resolved with PR #53932 (comment)

[Sophist-UK]

@jared-cannon Jared - your PR was merged - CONGRATULATIONS!!!!!