TwigBridge icon indicating copy to clipboard operation
TwigBridge copied to clipboard

Use lazy service initialization

Open aik099 opened this issue 11 years ago • 9 comments

Right now the Form class has FormBuilder $form argument in it's constructor. Because of this the HtmlServiceProvider is created (which is differed by the way) to inject that FromBuilder into Form class instance.

That unfortunately defeats the purpose of using deferred service providers in Laravel.

Maybe code can be changed in way, that TwigBridge will request Laravel services when it really needs them and not just create all of them at once regardless of fact if Twig templates even will be used at all.

Related to https://github.com/illuminate/html/pull/12#issuecomment-57605503

aik099 avatar Oct 02 '14 10:10 aik099

Hmm, tricky. Twig has to be loaded, because the Engine needs it and has to be registered for the Views. And before a template is rendered, extensions have to be loaded. So we could load all the dependancies for the extensions 'on the fly', but not sure how much that would save. Form/HTML aren't used on every requests, but most of them are (Config, Session, Auth etc), and the ones you don't ever use, shouldn't be used in the extensions. And form/html are probably used in a lot of files.

It would probably more benefitial to lazy-load the Twig engine itself, because that loads every extensions and Twig file on every requests, even when not needed.

barryvdh avatar Oct 02 '14 10:10 barryvdh

How does Blade solve this? I suspect that FormBuilder instance won't be created until I have Form:: in the *.blade.php file. I register all facades in the config/app.php so it knows that Form facade requires form service to live. Then a HtmlServiceProvider is created (because it's responsible for form service) and then actual FormBuilder instance is created and used to handle that original Form:: call.

So the Twig extension could use Facade to get hold of FormBuilder only when any of it's functions will be present on template. Instead it injects FormBuilder into constructor unconditionally even without knowledge if that FormBuilder will ever be used.

aik099 avatar Oct 02 '14 10:10 aik099

That unconditional constructor injection problem was one of the reasons (IMO) behind controller action dependency injection feature in Laravel 5. So you can have say public function show($id, FormBuilder $form) in your controller and $form argument will be injected by Container automatically.

aik099 avatar Oct 02 '14 10:10 aik099

You can use the TwigBridge\Extension\Loader\Facade\Caller which works the same way (Form.open() vs form_open()), instead of loading the form (and other) extensions.

Using the facades in extensions/service providers isn't really supposed to be done.

Regarding the method injection, not sure what that has to do with Twig extensions?

barryvdh avatar Oct 02 '14 10:10 barryvdh

Regarding the method injection, not sure what that has to do with Twig extensions?

I wrote about it to tell, that there is an alternative to constructor injection at least in controllers. Yes, this won't help us much in particular case.

aik099 avatar Oct 02 '14 10:10 aik099

You can use the TwigBridge\Extension\Loader\Facade\Caller which works the same way (Form.open() vs form_open()), instead of loading the form (and other) extensions.

Interesting, I'll try that. Are you using TwigBridge yourself?

aik099 avatar Oct 02 '14 10:10 aik099

It appears, that I need to declare all Facades twice (once in config/app.php in aliases array and in extensions.php of TwigBridge to make it work.

aik099 avatar Oct 02 '14 10:10 aik099

That is correct, because Twig doesn't really allow for static calls, without this kind of hacky extension. And yes I do use TwigBridge, as I'm one of the authors of this package. I also came up with the Facade Caller extension, but it still doesn't feel right ;)

barryvdh avatar Oct 02 '14 10:10 barryvdh

I've created PR: #153 where I'll populate all facades that have extensions for them. This way it's easier to get started.

aik099 avatar Oct 02 '14 11:10 aik099