-
Notifications
You must be signed in to change notification settings - Fork 3
Description
At the moment there are two similar systems handling the same task - Laravel's builtin Events and Actions API provided by Codice itself. It's not necessarily a bad thing since Actions allow for prioritizing, naming every registered callband and by thus deregistering them.
It does, however, result in code duplication and, what's even worse, there are situations where calling an Action is actually calling underlying Laravel Event, like in case of detecting route match:
// Route::matched() internally fires an Event
Route::matched(function ($objects) {
/**
* Executed on route matching
*
* @since 0.5.1
*
* @param \Illuminate\Routing\Route $route Route class object
* @param \Illuminate\Http\Request $request Object of current request
*/
Action::call('route.matched', [
'route' => $objects->route,
'request' => $objects->request,
]);
});It negatively affects performance (and readability) for no good reason.
Desired solution would be to use Events API directly from the Action class. It would allow to avoid manual conversion of Actions to Events and ensure all events fired by the Laravel are available for Codice plugins. Public Actions API should be kept to retain backwards compatibility and features like prioritizing and deregistering.
The difference between both solution is that in Codice event (actions) are referenced by string identifiers, while Laravel uses fully qualified classes' names for that. Preferred solution is to provide mapping between those identifiers and FQNs - still manual although much simpler to maintain, less error-prone and also allow for being independent of Laravel changes in this regard. Basically something like:
Illuminate\Routing\Events\RouteMatched::class => 'route.matched',Opinions are welcome.