[5.8] Application::registerConfiguredProviders - Load Order Change

[TerrePorter]

Why I am making this pull request:

History/Reasons

I wrote StringBladeCompiler which adds to the functionality of the View class. The original way I replaced the View component was to remove the ServiceProvider reference in the config/app.php file and register the package ServiceProvider, which set up my extended view classes as the base View class.

This worked fine until the ServiceProvider auto load feature was implemented. Vendor packages that are registered with autoload are loaded before vendor packages referenced in the config/app.php file.

A example of the issue, I was messing around with https://github.com/the-control-group/voyager, a admin package. That package requires this package, https://github.com/larapack/voyager-hooks, which registers a view namespace in the register function, #L40

Using my original method of replacing the View class, and the example above, this process will now cause an error as other package service providers load before mine. This results in the View class not existing, when the voyager-hooks register function tries to add a view namespace location. However, before the autoloading of service providers my process worked fine.

I have since updated my code to not need the removal of the original View registration in config/app.php. So this is not a "issue" for me anymore.

The However

I thought it was odd that vendor autoloaded service providers get priority over the vendor service providers I registered manually in the config/app.php file. I felt that the service providers that I specify in the config/app.php should get priority over any vendor package service provider that is autoloaded.

The pull request is to allow that ability.

The Existing Code

The existing code in Application::registerConfiguredProviders, does the following.

• It gets the providers from $this->config['app.providers'], splits them in to two groups. Those packages that start with Illuminate\\ and everything else.
• It next prepends, the AutoLoaded vendor packages to the everything else group.

This results in the following registration order for Service Providers:

array:38 [
  0 => "Illuminate\Auth\AuthServiceProvider"
  // trimed
  21 => "Illuminate\View\ViewServiceProvider"
  
  // autoloaded vendor packages
  22 => "Arrilot\Widgets\ServiceProvider"
  // trimed
  31 => "TCG\Voyager\Providers\VoyagerDummyServiceProvider"
  
  // config/app.php vendor and application providers.
  32 => "Wpb\String_Blade_Compiler\ViewServiceProvider"
  33 => "App\Providers\AppServiceProvider"
  34 => "App\Providers\AuthServiceProvider"
  35 => "App\Providers\EventServiceProvider"
  36 => "App\Providers\RouteServiceProvider"  
]

As you can see my directly registered package is not loaded until after all other vendor packages are loaded.

The code change

• It gets the providers from $this->config['app.providers'], splits them in to two groups. Those packages that start with Illuminate\\ and everything else.
• It then splits the everything else group extracting the App\\ service providers.
• It then reassembles the group to be Illuminate\\, config/app.php vendor, and App\\
• It next prepends, the AutoLoaded vendor packages to the App\\ group.

This results in the following registration order for Service Providers:

array:38 [
  0 => "Illuminate\Auth\AuthServiceProvider"
  // trimed
  21 => "Illuminate\View\ViewServiceProvider"
  
  // vendor packages registered in config/app.php
  22 => "Wpb\String_Blade_Compiler\ViewServiceProvider"
  
  // autoloaded vendor packages
  23 => "Arrilot\Widgets\ServiceProvider"
  // trimed
  32 => "TCG\Voyager\Providers\VoyagerDummyServiceProvider"
  
  // config/app.php application providers.
  33 => "App\Providers\AppServiceProvider"
  34 => "App\Providers\AuthServiceProvider"
  35 => "App\Providers\EventServiceProvider"
  36 => "App\Providers\RouteServiceProvider"
  37 => "App\Providers\CopyAssetsProvider"
]

As you can see the new registration order now give the vendor packages registered in config/app.php priority over the auto registered packages.

Sorry about the book, but I wanted to provide enough information to show why I felt this change would be of benefit.

Thanks for reading.

[fragkp]

Here u could use getNamespace 😃

Suggested change

	return Str::startsWith($provider, 'App\\');
	return Str::startsWith($provider, $this->getNamespace());

[crynobone]

Using my original method of replacing the View class, and the example above, this process will now cause an error as other package service providers load before mine.

I check both packages and failed to where where exactly did you replace View class. As far as I can see it only register view namespace, voyager did it correctly under boot() while voyager-hook attempt to do it under register() which is wrong.

[devcircus]

Seems like there's been a lot of discussion about this and several PRs, however I've not yet understood the need. This shouldn't be necessary.

[crynobone]

<?php

namespace Wpb\String_Blade_Compiler;

use Illuminate\Support\ServiceProvider;
use Wpb\String_Blade_Compiler\Compilers\StringBladeCompiler;
use Wpb\String_Blade_Compiler\Engines\CompilerEngine;

class StringBladeServiceProvider extends ServiceProvider
{
    /**
     * Register the service provider.
     *
     * @return void
     */
    public function register()
    {
        // include the package config
        $this->mergeConfigFrom(
            __DIR__.'/../config/blade.php', 'blade'
        );

        $this->app->afterResolving('view.engine.resolver', function ($resolver) {
            $this->registerStringBladeEngine($resolver);
        });
    }

    /**
     * Register the StringBlade engine implementation.
     *
     * @param  \Illuminate\View\Engines\EngineResolver  $resolver
     * @return void
     */
    public function registerStringBladeEngine($resolver)
    {
        $app = $this->app;

        // The Compiler engine requires an instance of the CompilerInterface, which in
        // this case will be the Blade compiler, so we'll first create the compiler
        // instance to pass into the engine so it can compile the views properly.
        $app->singleton('stringblade.compiler', function ($app) {
            $cache = $app['config']['view.compiled'];
            return new StringBladeCompiler($app['files'], $cache);
        });

        $resolver->register('stringblade', function () use ($app) {
            return new CompilerEngine($app['stringblade.compiler']);
        });
    }

    /**
     * Get the events that trigger this service provider to register.
     *
     * @return array
     */
    public function when()
    {
        return ['Illuminate\View\ViewServiceProvider'];
    }
}

This would simply be enough to handle what the @TerrePorter wanted to do in his package. There no reason to completely override the default Illuminate\View\ViewServiceProvider when Laravel already given you a way to properly extends it.

[TerrePorter]

@crynobone

Both packages? I am assuming you mean the voyager ones. The voyager packages to not replace View.

My package is a replacement for View, the replacement would happen when the person installing the StringBladeCompiler package added my package service provider in config/app.php. In my install instructions I had them commend out the original View service provider. I realize this was likely not needed. However, it didn't matter before as my service provider was the very next in line, right after the spot where the View service provider was, so I got loaded next.

Regarding the code change using when function. I didn't know about that function. I am not sure it existed when my package was originally created. So your saying, when Application::registerConfiguredProviders function runs the service provider classes are already instantiated? and the only the register function is just called at that time.

@devcircus
Perhaps it is not necessary, if your refereeing to the when function as the alternative. However, maybe the documentation should be updated, as I didn't know about the when function. And I don't see a reference to the function on the Service Provider page - https://laravel.com/docs/5.8/providers

[crynobone]

Disregard above comment. Didn't realise that Laravel have removed the functionality in 5.5 but keep the the registration of the event in Illuminate\Foundation\ProviderRepository

<?php

namespace Wpb\String_Blade_Compiler;

use Illuminate\Support\Facades\View;
use Illuminate\Support\ServiceProvider;
use Wpb\String_Blade_Compiler\Compilers\StringBladeCompiler;
use Wpb\String_Blade_Compiler\Engines\CompilerEngine;

class StringBladeServiceProvider extends ServiceProvider
{
    /**
     * Register the service provider.
     *
     * @return void
     */
    public function register()
    {
        // include the package config
        $this->mergeConfigFrom(
            __DIR__.'/../config/blade.php', 'blade'
        );

        View::resolved(function ($view) {
            $this->registerStringBladeEngine($view->getEngineResolver());
        });
    }

    /**
     * Register the StringBlade engine implementation.
     *
     * @param  \Illuminate\View\Engines\EngineResolver  $resolver
     * @return void
     */
    public function registerStringBladeEngine($resolver)
    {
        $app = $this->app;

        // The Compiler engine requires an instance of the CompilerInterface, which in
        // this case will be the Blade compiler, so we'll first create the compiler
        // instance to pass into the engine so it can compile the views properly.
        $app->singleton('stringblade.compiler', function ($app) {
            $cache = $app['config']['view.compiled'];
            return new StringBladeCompiler($app['files'], $cache);
        });

        $resolver->register('stringblade', function () use ($app) {
            return new CompilerEngine($app['stringblade.compiler']);
        });
    }
}

Facade::resolved() was introduce in #26824

[TerrePorter]

@crynobone what removed functionality are you referring to? the when function is in the service provider code,

framework/src/Illuminate/Support/ServiceProvider.php

Line 301 in dafef79

public function when()

[crynobone]

what removed functionality are you referring to?

Laravel still register event using when(), but it no longer fire/dispatch the event under Application::markAsRegistered().

Laravel 5.3 - https://github.com/laravel/framework/blob/5.3/src/Illuminate/Foundation/Application.php#L619-L626
Laravel 5.8 - https://github.com/laravel/framework/blob/5.8/src/Illuminate/Foundation/Application.php#L663-L674

Best approach now is to use Facade::resolved($callback) or $this->app->afterResolving('view', $callback).

[taylorotwell]

But, this is just reverses the scenario and possibly puts the problem on someone else. It's very much not recommended to depend on the registration order of service providers in any way. I think @crynobone is on the right track in offering alternative solutions.

[TerrePorter]

@crynobone
I looked in to the functions you referenced, but I of course couldn’t use them in the service provider, and I stepped through a page loading process and found that no files in the App\\ directory are loaded before the service providers are registered, which was annoying.

I discovered another function alternative. But the only place I found where I could precede the service provider registration process was in the bootstrap/app.php. I can add the code before the return $app; statement at the end of the file.

$app->resolving(Illuminate\Foundation\Bootstrap\RegisterProviders::class, function () use ($app) {
    // need? necessary? to be the first service provider - hell no, its just because I want to
    dump('I want to be first before any service provider listed in config/app.php');
    $app->register(\MyClass::class);
});

$app->resolving('view', function () use ($app) {
    dump('I want to be next in line after the view service provider');
    $app->register(\MyClass::class);
});

So technically, my proposed code change is not “needed”, as I can easily circumvent the service provider loading process now.

[TerrePorter]

@taylorotwell
I really don’t understand why this idea is such a bad idea.

But, this is just reverses the scenario and possibly puts the problem on someone else.

Perhaps, not sure how though. Maybe making a package that extends another package and the parent package is not registered yet, I guess...

It's very much not recommended to depend on the registration order of service providers in any way.

Sure, says the guy who has the service providers loading in a preset hard-coded order. (Illuminate\, Autoload, Config vendor and Config App\). I don’t see a ‘array random’ in there, so view is always going to be the last of the Illuminate providers loaded and before any of the autoloads – that is “dependable” to me.

I think @crynobone is on the right track in offering alternative solutions.

Yes, he was very helpful. I have a working alternative. Wrap my solution in to a "install" command with my package and the service load order means nothing.