-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Dependency Injection support with CONTAINER variable #221
Conversation
Here's an example of the configuration you could have to use such a DI Container in your
and the corresponding <?php
use DI\ContainerBuilder;
class Container extends \Prefab
{
/** @var \DI\Container */
protected $container;
public function get($name) {
return $this->getContainer()->get($name);
}
public function make($name, array $parameters = []) {
return $this->getContainer()->make($name, $parameters);
}
/**
* @return \DI\Container
*/
protected function getContainer() {
if (!$this->container) {
$containerBuilder = new ContainerBuilder();
$containerBuilder->addDefinitions('etc/di.php');
$this->container = $containerBuilder->build();
}
return $this->container;
}
} |
-1 Use auryn instead or implement as a plugin. |
Some notes:
|
Why would you need to create a method of wrapping a third party lib in F3? Just use your third party lib directly as intended? |
The PR enables F3's routing system to use any DIC to create the first controller. Without the PR it is currently not possible to inject dependencies when creating a controller automatically by the routing system. Currently there is only the option to use the |
@NeoVance As stated by @Rayne, using DI is not just like using any other third-party library. You need to bootstrap your class tree with it so that all dependencies can be resolved at once. In F3, the simplest and cleaner way is to do that when instanciating the controller (because it is the entry point of the user-code). My PR enables that. @Rayne Thanks for your feedback and the defense of my solution while I was away :) I really like your suggestions, I'll think about them and submit an improvement when I'm back from my vacations. As an additional note, my PR allows to use the DI library with every call to |
…lables support (fix)
I've updated my PR with @Rayne's suggestions. Below is a test file I've written to validate the implementation. Feel free to comment. require 'vendor/autoload.php';
require 'base.php';
class FakeClass {
public function test() {
return 'OK';
}
}
class FakeContainer {
public function has($class) {
return true;
}
public function get($class) {
return new $class;
}
}
class FakeContainerProxy extends Prefab {
public function get($class) {
return new $class;
}
}
$fw = \Base::instance();
$fw->set('CONTAINER', new FakeContainer());
$fw->call('FakeClass->test') == 'OK' or die('ERROR');
$fw->set('CONTAINER', function($class) { return new $class; });
$fw->call('FakeClass->test') == 'OK' or die('ERROR');
$fw->set('CONTAINER', 'FakeContainerProxy');
$fw->call('FakeClass->test') == 'OK' or die('ERROR'); |
Don't you think that it might be enough to just add a callable hook at the grab method to allow you to put your own container implementation there? Something simple as elseif ($this->exists('CONTAINER',$container))
$parts[1]=call_user_func_array($container,[$parts[1]]); Then you can adjust your container implementation the way you need it for your project: $fw = \Base::instance();
$container = new FakeContainer;
$fw->set('CONTAINER', function($class) use ($container) {
if (is_callable([$container,'has']) && $container->has($class)) //PSR11
$class=call_user_func([$container,'get'],$class);
elseif (is_callable($container))
$class=call_user_func($container,$class);
elseif (is_string($container) && is_subclass_of($container,'Prefab'))
$class=call_user_func($container.'::instance')->get($class);
return new $class;
}); |
@ikkez I could, but I'd really like to be able to define my |
Then what about this? elseif ($this->exists('CONTAINER',$container) && $container!=$func) {
if (is_string($container))
$container=$this->grab($container);
$parts[1]=call_user_func_array($container,[$parts[1]]);
} class FakeContainer {
public function has($class) {
return true;
}
public function get($class) {
return new $class;
}
public function load($class) {
if ($this->has($class)) {
$class=$this->get($class);
return new $class;
} else
user_error('class not found');
}
} $fw->set('CONTAINER','FakeContainer->load'); |
Well, I can't say it won't work, but nothing more. I think the goals I want to keep in mind here are as follows:
But I'm ok to adjust the implementation if needed. How about a vote? Then I'll make the necessary changes. |
My only intention here was:
It's still as easy as |
Hi there, I'd like to come to an accepted solution on that issue soon. It's been 7 months already, and my implementation has successfully passed the production tests so far. I'd like to hear from @bcosca if possible, and then make the final adjustments.
Thanks for your participation |
Well yeah I can understand that a simple way of defining the CONTAINER via config would be convenient. @xfra35 any thoughts about this topic? Actually I'm thinking about merging this as it is right now 🤔 |
I don't have a strong opinion on this. The solution proposed by @nanawel is simple and makes it easy to use external DI librairies. We just need to keep in mind that the framework could, in the future, embed its own service locator or DI container. Would be interesting to hear @bcosca's views. @nanawel, although your PR targets |
@xfra35 You're right, when using DI to instanciate the controller class (or any other class that uses In the cases of a PSR11-compatible or a In the case of a ...
elseif (is_callable($container))
$parts[1]=call_user_func($container,$parts[1]);
.... should be instead ...
elseif (is_callable($container))
$parts[1]=call_user_func($container,$parts[1],$args);
.... What do you think? |
Argh indeed :) Fixed. As for your remark, I agree. I'd even say that there's no reason to pass method arguments ( But we can't change that part without breaking BC, so we should flag this for |
Erf... I agree, reluctantly. So v4 it is. And I agree that |
Actually I don't see a break in BC right now for this.. but it could be that for v4, we'll introduce another solution, which then works differently than what is here supposed to be merged..that's true. was that your concerns, @xfra35 ? |
No, I'm just saying that the current PR won't fulfill the original need of @nanawel, which was to automatically inject dependencies into controllers. In order to fulfill it, we would need to prevent the framework from passing |
The current PR works for my need, but for that it does not inject |
Is that the case? I mean, for me it looks like the framework actually only passes |
That's the case:
IMHO |
One point regarding BC break if I may: it must be noticed that my PR does not break BC in its current form, because it only adds a new way of instanciating objects through There can't be existing implementations that use any old way, because... there is no old way using Of course, it means that converting an existing implementation to DI will require refactoring as the existing constructors might need |
Valid points. It will not break BC, because |
I played with it using Dice and it's pretty easy to setup: class A {
protected $b;
function __construct(B $b) {
$this->b = $b;
}
function get() {
return $this->b->data();
}
}
class B {
protected $data = 123;
function data() {
return $this->data;
}
}
$dice = new \Dice\Dice;
$f3->set('CONTAINER',function($class) use ($dice) {
return $dice->create($class);
});
echo $f3->call('A->get'); // 123 |
I'm merging this now because I can see the needs, especially for larger applications and using Dependency Injection for better decoupling of components. Having a support for this at the core level is a good thing and it's flexible enough to allow usage of any container solution you'd prefer. |
How can I use with controller, service and repossitory? |
@tuzoftware What do you mean by this? the CONTAINER is "just" an option to integrate an existing DIC into the F3 core, meaning when you use $f3->call, the "whatever" DI lib you use, is called to resolve dependencies. How to use the DI you choose, you have to read in their docs.. PHP-DI, Dice, auryn, just to name a few. |
@ikkez @nanawel please could any of you give further example of how Dice rules (addRules) fit into using this f3 CONTAINER option, using @ikkez dice example above. Also is then |
@eazuka just use the Dice rules as described in its manual.
No. What should it do? $f3->call is for calling a function or class method. If you just want to instantiate A and save it to the hive, it's |
I have been using those arguments in my controllers. So for anyone else reading this and trying to figure it out, the documentation example doesn't show it, but if you use a callable to wrap your container, then you get free backwards compatibility! Ex:
Note that second argument - $args is available there (as of 3.7 at least), and they're the same that F3 would pass if you weren't using a container. Now your controllers that were written as
will still receive the same arguments that they would without using a container - the framework, the route parameters with tokens resolved, and the actual callback string. This is because grab() does the following:
Note: it only works if you use a callable for the container. Using a PSR-11 or Prefab-based container directly will not pass the $args. You can wrap it in a callable and do what you need to to get the args setup though, depending on your container library. For new projects, it's probably better to let the container itself resolve arguments. But if you do need backward compatibility with existing code, it's very easy to get that. |
F3 works almost out-of-the-box with DI frameworks like PHP-DI.
The only thing it lacks is the DI "bootstrap" code that starts all the injection.
I found that the best way to achieve that is to add an alternative to the
\Base::call()
method. Currently it works like this (let's say to call\MyController->index
):\MyController
is a subclass of\Prefab
, we call\MyController::instance()->index
\MyController
and call the method directly on itWith the DI support I'm proposing, it's almost the same, but the 2nd alternative becomes the 3rd:
\MyController
is a subclass of\Prefab
, we call\MyController::instance()->index
CONTAINER
defined, we use it as a\Prefab
to retrieve an instance of the\MyController
via itsget()
method\MyController
and call the method directly on itPretty simple, isn't it?
With this I had no other F3-related thing to do to integrate PHP-DI on my application, and everything works the same.
If you're asking "Why
CONTAINER
?", it's just the generic term for the dependency injection main component.