Skip to content
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

Inform the application container about deferred providers before registering providers #38724

Closed
wants to merge 2 commits into from

Conversation

LukeTowers
Copy link
Contributor

This change informs the application container about the deferred providers before registering the eager loaded providers. This fixes the issue of the vague exception of "Facade root not set" when attempting to use a deferred service provider via a facade (such as Cache) during the registration process of eagerly loaded service providers. The application cannot resolve a deferred provider unless it knows it has been deferred, and this line is what informs it of the deferred providers.

This should be an entirely harmless change that will enable the use of already loaded deferred service providers (that a developer would assume could be used, like the Cache provider) during the registration process of eager loaded service providers.

I understand if this change won't be accepted, but if it won't be I would greatly appreciate learning more about what potential issues this could cause so that I can understand the service provider registration flow in more detail.

…stering providers

This change informs the application container about the deferred providers before registering the eager loaded providers. This fixes the issue of the vague exception of "Facade root not set" when attempting to use a deferred service provider via a facade (such as Cache) during the registration process of eagerly loaded service providers.
silly github editor...
@taylorotwell
Copy link
Member

It is not recommended to use any services in register. Only bind.

LukeTowers added a commit to wintercms/winter that referenced this pull request Dec 6, 2021
Fixes #350. Refs laravel/framework#38724. Final solution in v1.2 will be to extend the base Laravel Cache serviceprovider and make it no longer a deferred provider as well as consolidating the MemoryCache implementations currently present in QueryBuilder, Halcyon Models, and the Halcyon ServiceProvider into the Winter Cache service provider.
bennothommo pushed a commit to wintercms/wn-system-module that referenced this pull request Dec 9, 2021
Fixes #350. Refs laravel/framework#38724. Final solution in v1.2 will be to extend the base Laravel Cache serviceprovider and make it no longer a deferred provider as well as consolidating the MemoryCache implementations currently present in QueryBuilder, Halcyon Models, and the Halcyon ServiceProvider into the Winter Cache service provider.
LukeTowers pushed a commit to wintercms/storm that referenced this pull request Jun 25, 2022
Reverts dff4e6e
Partially reverts 6d95046

Implements laravel/framework#38724 into the Winter codebase, extending the base Laravel ProviderRepository class and overriding the load method.

This will override the base provider loading functionality to record deferred providers before registering any eager loaded providers. This allows eager loaded providers to use deferred provider functionality (such as the Cache provider). In this method, this will technically make those providers eager loaded as well, but these providers will be deferred if no other provider uses them in registration.
@LukeTowers LukeTowers deleted the patch-1 branch June 25, 2022 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants