Resolves view factory only at services' construction time

[MatteoOreficeIT]

Signed-off-by: Matteo Orefice [email protected]

This change allows to register Barryvdh\LaravelIdeHelper\IdeHelperServiceProvider in any order in app.php, I discovered it cannot be registered before Illuminate\Filesystem\FilesystemServiceProvider so the documentation is not really in sync because it throws an error if we put Barryvdh\LaravelIdeHelper\IdeHelperServiceProvider at the top

Based on my actual knowledge trying to resolve object via the container is not allowed before every provider's register() method has been completed

[mfn]

PR makes sense to me, also tested it locally.

However the change in createLocalViewFactory looks like a micro-optimization .. I suggest to leave the method as is.

Further, I think this can be removed from the boot() method:

        if ($this->app->has('view')) {
            $viewPath = __DIR__ . '/../resources/views';
            $this->loadViewsFrom($viewPath, 'ide-helper');
        }

When this code was rewritten to use a standalone view, it wasn't removed but the package also isn't referencing it 🤷‍♀️

[barryvdh]

Do we even need to explicitly list the factories? We can just use DI now right?

[mfn]

Probably not because of #475

This should safeguard from app-dedicated overrides to the View Factory used in-app.

People use dedicated view factories not supporting php views, thus packages assuming php templates work (like this) break, thus a custom view is necessary.

[mfn]

@MatteoOreficeIT do you still need the PR?

[MatteoOreficeIT]

undo of createLocalViewFactory optimization that is not strictly required

[MatteoOreficeIT]

@MatteoOreficeIT do you still need the PR?

Yes, this code break the Laravel register/boot event sequence, my only remark is about the time createLocalViewFactory() is called, we can build the independent VF as we need even after, but not in register event. It could also be defined with DI but I don't need to inject anything of different

[mfn]

Some tests are failing however

[MatteoOreficeIT]

Hi, I will try to fix in next days

[barryvdh]

Closing due to inactivity