laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.64k stars 11.04k forks source link

[12.x] Enable listening to multiple Eloquent events #53562

Open istiak-tridip opened 3 days ago

istiak-tridip commented 3 days ago

Description

This PR introduces a new Model::listen method, allowing developers to listen to multiple Eloquent events at once.

This enhancement improves the developer experience and aligns with the existing Event::listen behavior. Additionally, it introduces support for wildcard event listening for Eloquent models.

Currently, performing the same action for multiple events looks like this:

Categories::created(function () {
    Cache::forget('categories');
});

Categories::deleted(function () {
    Cache::forget('categories');
});

With This PR:

Categories::listen(['created', 'deleted'], function () {
    Cache::forget('categories');
});

// Listen to all Eloquent events for the model
Categories::listen(function () {
    // Do something
});

Targeting the master branch based on @taylorotwell's suggestion.

Diddyy commented 3 days ago

This looks like it'll clean up alot of my code base!

chr15k commented 3 days ago

hey @istiak-tridip is there a use case for listening on all events?

istiak-tridip commented 2 days ago

@chr15k One of the goals of this PR was to align with Event::listen, which supports wildcard event listening, so I included it here as well. While I personally don’t have a specific use case for wildcard listeners, here are a few possible scenarios:

Happy to hear your thoughts!

chr15k commented 2 days ago

@chr15k One of the goals of this PR was to align with Event::listen, which supports wildcard event listening, so I included it here as well. While I personally don’t have a specific use case for wildcard listeners, here are a few possible scenarios:

  • To listen to all events except specific ones (e.g., exclude certain events).
  • Perform actions when a model is booting or booted.
  • Track model activity or usage through comprehensive event logging.

Happy to hear your thoughts!

@istiak-tridip thanks, just to be more specific; my understanding is in order to achieve this with Event::listen you'd need to explicitly pass the wildcard, which shows intent:

  Event::listen('*', function () {
      Cache::forget('categories');
  });

I don't believe that you can pass a callback as first param with Event::listen without declaring the specific Event class as the closure param:

for example, this will not work:

Event::listen(function () {
    Cache::forget('categories');
});

But this will:

Event::listen(function (CategoryDeletedEvent $event) {
    Cache::forget('categories');
});

However, the following does work, which IMO is unexpected if going by the above behaviour, and the underlying '*' wildcard that's in effect, is hidden from the dev:

Categories::listen(function () {
   Cache::forget('categories');
});

Let me know your thoughts.

Cheers!

istiak-tridip commented 2 days ago

@chr15k Huh, I can't believe I missed that. Thanks for pointing it out! 🙌 What syntax would you suggest?

Based on my current implementation, developers can still use * to listen to wildcard events. Since model events are string and not class, I don't think we can replicate the Event::listen closure behavior (maybe I'm wrong 🤔?).

With that in mind, I propose keeping Model::listen(fn () => dump('...')) as syntax sugar for the wildcard listener to enhance the developer experience. However, I'm open to changes and would love to hear feedback from @taylorotwell or others.

chr15k commented 2 days ago

@istiak-tridip yeh I'd like to hear others' opinions on this for sure, as I can see this as something I'd use in my projects and agree it would be quite useful to add a catch-all for events on a model. I suppose all I'm wondering is if the wildcard should be set explicitly or not, for the purpose of clearer intent. If so, a straightforward approach could be to disallow the first parameter from accepting a closure.