cviebrock / eloquent-taggable

Easily add the ability to tag your Eloquent models in Laravel.
MIT License
537 stars 72 forks source link

Events throwing exceptions on Lumen #103

Closed judgej closed 3 years ago

judgej commented 5 years ago

Using Lumen 5.8

When tagging or untagging a model, the events throw an exception:

Missing trait Illuminate\Foundation\Events\Dispatchable

Lumen dispatches events differently to how Laravel does so. Apart from this, everything else works fine on Lumen.

To work around this, I have forked the repo to comment out the event dispatching:

https://github.com/consilience/eloquent-taggable/commit/f9dd2f98edfbddd92c84a8c3d558897f6ee5700f

This isn't an ideal solution, as it does not get to the root of the problem, no longer supports the events (which we don't need anyway at this time) and is yet another fork to maintain.

Ultimate solution would be to support both Lumen and Laravel. Alternatively, being able to switch off the events by configuration would fix the necessity for the fork, but still does not fix the real problem.

A rather old post here gives some hints on another solution - firing different events in different ways depending on what kernel classes are available:

https://laracasts.com/discuss/channels/lumen/now-that-laravel-54-no-longer-support-kernelhandled-events-what-do-we-do-in-lumen

Is Lumen support something you would like to continue? Or am I on my own with my fork?

cviebrock commented 4 years ago

Ideally, I'd like to support both Lumen and Laravel. The Lumen 6.x docs don't seem to indicate any hint that this would be a problem, and that dispatching the event with:

event(new ModelTagged($this, $tags));

should "work". But I'm going to take your word that it doesn't work over the docs!

I'll try and spend some time solving this, but to be honest I'm not going to have much time over the next little while. If you figure out a solution and put together a PR, I'd be more than happy to implement it.

judgej commented 4 years ago

Thank you for the update.

We have simply forked the package for now (also under time pressure) and commented out the two event lines, since we don't use the events. If L6 supports taggables across both platforms, then I guess we don't want to spend too much time fixing it for compatibility if that is unnecessary for L6.

I wonder if the solution in the meantime is a simple shim? I'll have a think about how that could be done, and if it can, then that would allow Lumen 5.x use without too much effort. The shim does not even have to be in this package - all it needs to do is provide the missing trait. Maybe that's something other packages have solved?

I'll post any further updates, but that will likely be towards the end of next week (mid-to-late October) at the earliest.

judgej commented 4 years ago

Okay, as I was working on this bit anyway, I tried this shim on Lumen 5.8, and it seems to work when I remove my commented-out hacks:

/bootstrap/shims.php

<?php

/**
 * Shims for classes and traits missing from Lumen that some
 * packages need.
 */

namespace Illuminate\Foundation\Events {

    // cviebrock/eloquent-taggable needs this.

    if (! trait_exists(Dispatchable::class)) {
        trait Dispatchable
        {
            /**
             * Dispatch the event with the given arguments.
             *
             * @return void
             */
            public static function dispatch()
            {
                return event(new static(...func_get_args()));
            }

            /**
             * Broadcast the event with the given arguments.
             *
             * @return \Illuminate\Broadcasting\PendingBroadcast
             */
            public static function broadcast()
            {
                return broadcast(new static(...func_get_args()));
            }
        }
    }
}

In bootstrap/app.php at the end I include the shim script:

include __DIR__ . '/shims.php';

There may be some surprises I haven't encountered yet, but this allows me to tag and untag without the "Fatal error: Trait 'Illuminate\Foundation\Events\Dispatchable' not found" exception being thrown.

This is also an older shim from Laravel 5.6 I think, so there may be a couple more methods in it now. In other words, rough around the edges, but hopefully it's a start.

cviebrock commented 3 years ago

Closing this old ticket. I'm sorry I never spent time looking into what the issue was or how to fix it. And I'm not sure if it still exists with current versions of the package, Laravel, or Lumen. If so, and if this is still an issue for you, feel free to re-open with updated details.