PHP-DI / Slim-Bridge

PHP-DI integration with the Slim framework
http://php-di.org/doc/frameworks/slim.html
MIT License
175 stars 38 forks source link

Adding dependency injection on middleware. #11

Closed sicet7 closed 6 years ago

sicet7 commented 8 years ago

I have been using this for a couple of months now and i have not run into any issues with it, but it might not be the best way to implement this feature but it sure does work.

Added it in a separate method to make compatibility issues less common.

If there is something wrong with this code or there is improvements to be done please do contact me because i use this myself so any optimizations is appreciated

mnapoli commented 8 years ago

Hi! Sorry for taking so long to answer, quite a busy week. I think this is a good idea, however maybe it would be better if all middlewares were registered the same way? Also using the "foundHandler" might not be reliable, I'm not sure if I'm right or wrong but I think this handler is meant to be used for controllers, not middlewares, so we could get unexpected behaviors out of this.

sicet7 commented 8 years ago

No worries.

I see what you mean as it is the "ControllerInvoker" i use a simple fix would be to make a copy of that and call it "MiddlewareInvoker" or somthing and simply remove the "InvocationStrategyInterface" from the class and the "$routeArguments" from the "__invoke" magic method and from there on out i cannot see what unexpected behaviors would occur.

Also, replacing the "addMiddleware" method with this method would work but that would also require a replacement of the "add" method, as it wraps the object in a class called "DeferredCallable" witch will not work. The "add" method replacement is no problem as it simply need to call the new "addMiddleware".

Will get to work on the things explained above :smile:

sicet7 commented 8 years ago

I have been looking into this and i cannot seem to locate where there would be a problem with the "foundHandler". The setup of the invoker is ideal, the only thing is maybe to make a separate instance for the middleware but that is about it.