Carbon: [Laravel] Verify event dispatcher against its contract

[lorisleiva]

Hi there 👋

I created a library that decorates the Event dispatcher of Laravel and I'm having some issue with this piece of code.

https://github.com/briannesbitt/Carbon/blob/702dcd374f960d752ab999543588350e13c5e463/src/Carbon/Laravel/ServiceProvider.php#L22

It verifies that $app['events'] is an instance of Illuminate\Events\Dispatcher when I think it should verify it using the Illuminate\Contracts\Events\Dispatcher interface (which forces the implementation of the listen method).

Would it be okay for me to write a PR that makes this change? I could simply add a third OR expression to ensure that it will not be a breaking change.

// Before
if ($events instanceof EventDispatcher || $events instanceof Dispatcher) {}

// After
if ($events instanceof EventDispatcher || $events instanceof Dispatcher || $events instanceof DispatcherContract) {}

There might be a reason that I'm not aware of for the current implementation but I think verifying against a contract is a lot more "OOP-proof" then verifying against a certain implementation of that contract. This will allow Laravel libraries to decorate the event dispatcher without side effects.

Thank you for your time and your amazing library that I use on a daily basis. ✨

[kylekatarnls]

Hello,

It's OK, I will be glad to merge your PR. The goal is to take the locale of the $this->app['translator'] and use it as Carbon default locale as often as it changes.

The interface does not guarantee the listen method handle properly string, closure parameters, but it's OK for me to assume it's implemented following the PHPDoc code block:

    /**
     * Register an event listener with the dispatcher.
     *
     * @param  string|array  $events
     * @param  mixed  $listener
     * @return void
     */
    public function listen($events, $listener);

And so should not break with values we currently pass to it. Let's accept all Illuminate\Contracts\Events\Dispatcher implementations!

[lorisleiva]

Awesome, thanks!

You can expect a PR from me by the end of today. 🙂

[kylekatarnls]

Merged, will be in next minor release.