laravel
Allow precognition dispatchers to be bound by name for overriding
Hi, I'm working on a PR for Laravel Actions with the goal of enabling Precognition for Actions, as I'd like to start using Precognition within Actions. While I've made this work with an extension of the HandlePrecognitiveRequests middleware for backwards compatibility that people would have to remember to use over the built in one, this is a cleaner way of doing and enables It Just Works™ compatibility.
This change is what allows Spice-King/laravel-actions@4c4720ac2123466f7dc444733e71ea22c9f889bb to make it seamless. I only really need ControllerDispatcher done for what I've done, but there was no compelling reason to not make them match and CallableDispatcher might be needed solve how WithAttributes functions without FormRequests.
I don't know what this fixes for you? You can still override them even without this change?
Hi Taylor, thanks for your time on this and the Laravel framework as a whole.
It changes how the Dispatcher instance is instantiated, rather than a callback closure where it returns a fixed result of an instance and ends the resolution there, it shifts the creation back into the Container's system of resolution where I can say a more specific Dispatcher is valid. I could run something like the following code in a package somewhere appropriate (a Service or mid resolution of a route's action) and then not have to question if the developer tagged their route with 'precognitive'/HandlePrecognitiveRequests::class by trying to inspect an incomplete stack of middlewares from multiple places.
app()->bind(PrecognitionControllerDispatcher::class, MyPackagePrecognitionControllerDispatcher::class);
While I can (and do, more for backwards compatibility and testing on older Laravel versions) have a custom version of the middleware in the PR for Laravel Actions which would solve this, it felt like bad DX in needing to remember to use the specific correct version when needed, something that would be worse if another package also need to do the same thing. I wanted to improve the DX to not require it and be more eloquent and inline with how Laravel's magic works.
Is there some considerations, like performance, I've overlooked that would be relevant here in making the Container class evaluate bindings recursively? It seemed like a well written and performant enough chunk of code to me, or at least well enough to not be hot spot needing to be used sparingly.