Implement DeferrableProvider

[kon-shou]

Related issue ＆ pr

#898 ( I cant understand this issue why IdeHelperServiceProvider is not deferd, so excuse me if I am mistaken ... )
#773

Problem Summary

When FilesystemServiceProvider is customized, laravel can't start because of follow error.

[2020-04-13 18:39:35] local.ERROR: Target class [files] does not exist. {"exception":"[object] (Illuminate\\Contracts\\Container\\BindingResolutionException(code: 0): Target class [files] does not exist. at /srv/vendor/laravel/framework/src/Illuminate/Container/Container.php:805, ReflectionException(code: -1): Class files does not exist at /srv/vendor/laravel/framework/src/Illuminate/Container/Container.php:803)
[stacktrace]
#0 /srv/vendor/laravel/framework/src/Illuminate/Container/Container.php(681): Illuminate\\Container\\Container->build('files')
#1 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(785): Illuminate\\Container\\Container->resolve('files', Array, true)
#2 /srv/vendor/laravel/framework/src/Illuminate/Container/Container.php(629): Illuminate\\Foundation\\Application->resolve('files', Array)
#3 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(770): Illuminate\\Container\\Container->make('files', Array)
#4 /srv/vendor/laravel/framework/src/Illuminate/Container/Container.php(1245): Illuminate\\Foundation\\Application->make('files')
#5 /srv/vendor/barryvdh/laravel-ide-helper/src/IdeHelperServiceProvider.php(121): Illuminate\\Container\\Container->offsetGet('files')
#6 /srv/vendor/barryvdh/laravel-ide-helper/src/IdeHelperServiceProvider.php(64): Barryvdh\\LaravelIdeHelper\\IdeHelperServiceProvider->createLocalViewFactory()
#7 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(616): Barryvdh\\LaravelIdeHelper\\IdeHelperServiceProvider->register()
#8 /srv/vendor/laravel/framework/src/Illuminate/Foundation/ProviderRepository.php(75): Illuminate\\Foundation\\Application->register(Object(Barryvdh\\LaravelIdeHelper\\IdeHelperServiceProvider))
#9 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(593): Illuminate\\Foundation\\ProviderRepository->load(Array)
#10 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Bootstrap/RegisterProviders.php(17): Illuminate\\Foundation\\Application->registerConfiguredProviders()
#11 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(219): Illuminate\\Foundation\\Bootstrap\\RegisterProviders->bootstrap(Object(Illuminate\\Foundation\\Application))
#12 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(320): Illuminate\\Foundation\\Application->bootstrapWith(Array)
#13 /srv/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(129): Illuminate\\Foundation\\Console\\Kernel->bootstrap()
#14 /srv/artisan(37): Illuminate\\Foundation\\Console\\Kernel->handle(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
#15 {main}
"}                           

• PHP7.2
• Laravel v6.x

Problem Detail

Laravel load config file to be registered.
https://github.com/laravel/framework/blob/e4d49ff7c1fbd061a7f60831e515c95b996248a6/src/Illuminate/Foundation/Application.php#L583-L594

The order of above $providers is

1. Illuminate\\xxx (ex: Illuminate\\Auth\\AuthServiceProvider )
2. package's service provider (ex: Barryvdh\\LaravelIdeHelper\\IdeHelperServiceProvider )
3. Customized service provider (ex: App\\Providers\\FilesystemServiceProvider )

Then, IdeHelperServiceProvider->register() , createLocalViewFactory() are fired, $this->app['files'] is missing, laravel can't start.

(Because app['files'] means App\\Providers\\FilesystemServiceProvider , and this has not been loaded. )

Solution

1. IdeHelperServiceProvider should be deffer service provider (this pull request)
2. IdeHelperServiceProvider should not load other container at register() , in this case $this->app['files'] (Resolves view factory only at services' construction time #773)

[mfn]

I see two issues with this:

• the interface doesn't exist and won't work for Laravel < 5.8
  At the moment this library still supports 5.5!

• According to https://laravel.com/docs/7.x/providers#deferred-providers it is

  If your provider is only registering bindings

  But this isn't true: there's a boot() method because, well, the library does other things.

You're referencing #773 => does that solution work you or why did you chose another approach?

[barryvdh]

Does the console run all providers, or should we list the commands? Bumping to 5.8 is not a problem I guess.

[mfn]

Does the console run all providers, or should we list the commands? Bumping to 5.8 is not a problem I guess.

It's really a strange thing to me, I tried to make sense here 😄

Because the current ServiceProvider has a provides, even if you make it deferred, it's boot() method is called!
(this happens in \Illuminate\Foundation\Application::loadDeferredProviders which is also exactly documented as "Load and boot all of the remaining deferred providers")

Interestingly, if one would not provide a provides(), then yes, boot() also wouldn't be called.

And now since there is a provides(), there's something else interesting:

• the current provides only lists 2 out of 4 registered
• but since having a provides also seems to trigger a boot, all commands are in fact properly registered and things just work

(I'm a bit confused because I thought "deferred" means only the register() method is ever invoked and not the boot(), but hey 🤷‍♀️)

So my TL;DR is (from observation, not understanding): everything will work correctly (and supports for 5.5ends with this).

I guess it fixes it for OP because it simply loads it much later in the bootstrapping and then it fixes the override problem.

If we all agree "it's a go", this PR should be adapted:

• properly return all registered commands in provides
• adapt composer.json version constraints (remove Laravel 5.5, orchestra/testbench 3 I believe)
• bump minimum PHP version to 7.1.3 (it's the base L 5.8)
• adapt .travis.yml
• (I hope I didn't forget anything)

I'm also fine just merging and making a follow-up PR for the "cleanup" of course 😄

[barryvdh]

@kon-shou do you want to fix the the listed suggestions or should @mfn create a follow-up PR on this (in that case just create a new PR using the original commit)

[LukeTowers]

@mfn could you create a follow-up PR?

[mfn]

Note: the stance on 5.5 was changed

Here, from #914 (comment)

Bumping to 5.8 is not a problem I guess.

But #971 (comment) was declined (the reasons where others, but outcome is the same).

Bottom-line: with the current organization of the code, unless we decide to drop 5.5, we can't merge this PR at the moment.

Sad part: we've test coverage but most tests only are for L6+ simply because 5.5 is so old and I encountered lots of issues when I created e.g. the model phpdoc tests. This also means such a change will currently not break in CI 😢 , i.e. will go unnoticed unless manually tested.

[barryvdh]

If lost issues have been fixed we could probably just tag this and drop 5.x support from here on.

[mfn]

No objections from my side 🚀

[mfn]

@kon-shou I took the liberate and updated with master / resolved conflicts

@barryvdh 5.5 is gone, so I guess we'll be good here 🤞

[kon-shou]

Thanks for rebasing!, and sorry for response later.
(I concentrated on my project's problem which this PR should resolve, and I missed ...)

After PR submitted, I sorved the problem by I clone #773 file changes (IdeHelperServiceProvider.php) in my project, so Now this PR is no need for me.

But, It is good news that dropping laravel 5.5 👍
I’m looking forward to remove "my project's cloned" IdeHelperServiceProvider.php!

[mfn]

Thanks for getting back and no problem.

Eventually we'll work our way through to the other PR 😅