Skip to content

Conversation

@willrowe
Copy link
Contributor

@willrowe willrowe commented Apr 5, 2025

This delays the observe call in the boot method for the HasEvents trait until after the model has finished booting so that any custom events registered by other traits are available to be matched up to observer class methods.

This fixes the initial issue that was discussed in #53607

willrowe added 3 commits April 5, 2025 16:46
- This ensure that all custom events have been registered by any traits first.
- It also prevents a new instance of the model from being created by the call to `observe` too early, since an instance created before booting has finished would be in an inconsistent state due to not all of the trait boot methods being run yet and not all of the initializer methods would be registered yet, so they would not be run either.
@willrowe willrowe marked this pull request as draft April 5, 2025 21:11
@willrowe
Copy link
Contributor Author

willrowe commented Apr 5, 2025

There are some funky issues with the tests and listening for the booted event. I will have to take another look on Monday. Setting as draft for now.

@willrowe
Copy link
Contributor Author

willrowe commented Apr 7, 2025

The complexity introduced to testing by using the booted event in order to boot the events was not acceptable. When I was initially adding that listener it did give me pause since events relying on the event dispatcher seemed brittle.

A new whenBooted static method has been added to models, which can be used by traits to defer until after all other traits have been booted.

@willrowe willrowe marked this pull request as ready for review April 7, 2025 16:21
@willrowe willrowe force-pushed the hotfix/observable-attribute-with-trait branch from 06aad07 to 60731aa Compare April 7, 2025 16:22
@taylorotwell taylorotwell merged commit a750ae5 into laravel:12.x Apr 7, 2025
58 checks passed
@willrowe willrowe deleted the hotfix/observable-attribute-with-trait branch April 7, 2025 21:37
@azzazkhan
Copy link

@willrowe I'm not sure, but I guess this broke the functionality of defining magic methods on traits for Eloquent models, e.g., boot{TraitName}.

@willrowe
Copy link
Contributor Author

@azzazkhan how so?

@tom-on-the-internet
Copy link

Leaving a comment in case this helps someone else with their Laravel 12 upgrade.

This change makes sense and it solves another issue.

But, when upgrading to Laravel 12, this change caused a change in our business logic.

We expect the event hooks set by ObservedBy to run before event hooks set by a bootable Trait.

Previously, in our trait, we had

    protected static function bootMyTrait(): void
    {
        static::deleting(function ($model): void {
            // logic here
        });
    }

With this change, the above is now registered before our ObservedBy hooks. So, our trait's deleting runs first, which is not what we want.

To fix this, I've used the new static::whenBooted

    protected static function bootMyTrait(): void
    {
        static::whenBooted(
            fn () => static::deleting(function ($model): void {
                // logic here
            })
        );
    }

Hopefully this helps someone.

@willrowe
Copy link
Contributor Author

willrowe commented Jul 17, 2025

@tom-on-the-internet that is something I did not think of and makes this a bit of a breaking change. I'm wondering if it would be worth proxying registerModelEvent through whenBooted to restore the original order? I guess it depends on if it makes more sense for a specific order or for it to be intentionally unspecified so order should not be relied upon?

@willrowe
Copy link
Contributor Author

On second thought, it's probably best to leave it as-is. It does not look like the boot order of traits is specified, so it shouldn't be relied upon.

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.

4 participants