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

[5.5] Load app.providers before auto discovered providers #19652

Closed
Omranic opened this issue Jun 17, 2017 · 2 comments
Closed

[5.5] Load app.providers before auto discovered providers #19652

Omranic opened this issue Jun 17, 2017 · 2 comments

Comments

@Omranic
Copy link
Contributor

Omranic commented Jun 17, 2017

  • Laravel Version: 5.5-dev
  • PHP Version: 7.1.3

Description:

Related to #19457
The current behavior of loading app.providers after the auto discovered providers is causing some conflicts with other Laravel features (model observers)

Steps To Reproduce:

On a clean and fresh Laravel 5.5 installation..

  1. Create a new package that has the following:
    a. A Model
    b. A Model Observer
    c. Auto-Discovered Service Provider
  2. Now as per Laravel's docs we'll register the model observer in the package's Auto-Discovered service provider's boot method.
  3. Try now to do any action that trigger any observed model events, and you'll be surprised that nothing will happen. The model observer is useless and here's why in a pseudo workflow:
    PackageServiceProvider::boot() -> Model::observe() -> Model::registerModelEvent() -> isset(static::$dispatcher) -> static::$dispatcher == null tada!

Why static::$dispatcher is null, because the Auto-Discovered package's service provider, that register the model observer, which requires the event dispatcher, is registered first before Laravel's Illuminate\Database\DatabaseServiceProvider where the event dispatcher is being injected to the model through Model::setEventDispatcher. So in short, the model listeners never registered!

This is a clean and simple implementation of few basic Laravel features, that doesn't override any core functionality, and I can't see any reason not to use the Auto-Discovery feature. What do you think, any thoughts..? @taylorotwell @driesvints

Suggested solution

Load app.providers before the auto discovered providers.

@devcircus
Copy link
Contributor

Looks like #19646 addresses this.

@driesvints
Copy link
Member

Yeah #19646 should fix this.

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

No branches or pull requests

4 participants