Framework: Laravel 5.7 initializeTraits breaks models that override the boot method

Created on 5 Sep 2018  路  4Comments  路  Source: laravel/framework

  • 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

Most helpful comment

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: https://github.com/laravel/framework/blob/6a8ab1356b5497c7a15f6d12c682e5a53409df77/src/Illuminate/Database/Eloquent/Model.php#L165

All 4 comments

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: https://github.com/laravel/framework/blob/6a8ab1356b5497c7a15f6d12c682e5a53409df77/src/Illuminate/Database/Eloquent/Model.php#L165

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

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.

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

Was this page helpful?
0 / 5 - 0 ratings