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

Laravel 5.7 initializeTraits breaks models that override the boot method #25455

Closed
paras-malhotra opened this issue Sep 5, 2018 · 4 comments
Closed

Comments

@paras-malhotra
Copy link
Contributor

  • Laravel Version: 5.7.1
  • PHP Version: 7.1.18
  • Database Driver & Version: MySQL 5.7.19

Description:

This PR assumes all models call the Model::boot static method. There may be some implementations where the parent boot method is not called. That results in an error Undefined index [class name] in the initializeTraits method in the following line:

foreach (static::$traitInitializers[static::class] as $method) {

I'm not sure if it's expected that all models always call the Model::boot method but Laravel 5.7 certainly will break apps that don't do that.

Steps To Reproduce:

Override the public static function boot method in any model in Laravel and do not call the parent::boot method. Probably can check with the User model and visit any page - the following error will pop up: Undefined index App\User

@stayallive
Copy link
Contributor

stayallive commented Sep 5, 2018

If you are overriding the boot method but not calling the parent your not overriding it correctly, it is doing work that the framework depends on.

If you check some example in the Laravel docs you will also notice parent::boot() is always called.

Example: https://laravel.com/docs/5.7/eloquent#global-scopes (scroll little further to "Applying Global Scopes")

Unless you are overriding the constructor the boot method will always be called:

$this->bootIfNotBooted();

@paras-malhotra
Copy link
Contributor Author

I understand. However, this does break all apps migrating to 5.7 that don't override the parent::boot.

I see three possible decision points for this issue:

  1. Ignore and close this issue - if it breaks the app, the app owner must correctly call parent::boot
  2. Add an if(isset(static::$traitInitializers[static::class])) condition to avoid breakages
  3. Add a note to the Laravel docs / upgrade guide to ensure parent::boot is always called

@stayallive
Copy link
Contributor

To be clear, this does break all apps migrating to 5.7 that don't override the parent::boot. not sure if you ment this exactly or maybe it's an language thing and i'm misunderstanding.

But for clarity, you are saying that if you do:

class Entity extends \Illuminate\Database\Eloquent\Model 
{
    public static boot()
    {
         // some logic
    }
}

Instead of:

class Entity extends \Illuminate\Database\Eloquent\Model 
{
    public static boot()
    {
         parent::boot();

         // some logic
    }
}

it breaks any 5.7 application. Which I would say is very correct.

Not calling parent::boot() will also cause traits having an bootTraitName method to not be called. So IMO option 1/3 are the only correct options.

@paras-malhotra
Copy link
Contributor Author

Yes @stayallive that's what I meant. I'll close this issue in that case

snipe added a commit to snipe/snipe-it that referenced this issue Sep 6, 2018
This was causing the error: Undefined index: App\Models\CustomField (View: /Users/snipe/Sites/snipe-it/snipe-it/resources/views/hardware/index.blade.php)

and pointed to the Eloquent model library method:

```
protected function initializeTraits()
    {
        foreach (static::$traitInitializers[static::class] as $method) {
            $this->{$method}();
        }
    }
```

Thanks to laravel/framework#25455 for the clue.
gregory-claeyssens pushed a commit to gregory-claeyssens/model-locking that referenced this issue Jan 9, 2019
called parent::boot inside the boot method of ModelLock.php 
see: laravel/framework#25455
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

2 participants