Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Laravel 11: Autodiscovery of events is not working anymore #81

Closed
Wulfheart opened this issue Mar 24, 2024 · 13 comments
Closed

Laravel 11: Autodiscovery of events is not working anymore #81

Wulfheart opened this issue Mar 24, 2024 · 13 comments

Comments

@Wulfheart
Copy link
Contributor

Wulfheart commented Mar 24, 2024

The ModularEventServiceProvider calls shouldDiscoverEvents. However, with the new structure it is

public function shouldDiscoverEvents()
{
    return get_class($this) === __CLASS__;
}

which will always evaluate to false in an inherited class. This means that

public function shouldDiscoverEvents()
{
// We'll enable event discovery if it's enabled in the app namespace
return collect($this->app->getProviders(EventServiceProvider::class))
->filter(fn (EventServiceProvider $provider) => str_starts_with(get_class($provider), $this->app->getNamespace()))
->contains(fn (EventServiceProvider $provider) => $provider->shouldDiscoverEvents());
}

will always be false.

@Wulfheart
Copy link
Contributor Author

Going forward I would like to introduce a fix for it. However, I am not sure what the desired behavior will be as there is no way anymore to determine if autloading has been manually enabled or disabled. I guess, the best way forward would be a config value in the package where it can be enabled or disabled. What do you think?

@inxilpro
Copy link
Contributor

@Wulfheart I'm trying to confirm on my end… but it seems like the ModularEventServiceProvider might break event discovery in general in Laravel 11. Not 100% sure, but it kinda looks like if you register any provider that extends the Laravel EventServiceProvider, that will cause Laravel to not register the default one…

@mra-dev
Copy link

mra-dev commented Apr 3, 2024

@inxilpro Isn't there any workaround yet ?

@Wulfheart
Copy link
Contributor Author

@mra-dev I have one in a branch but that is not a durable solution.

@mra-dev
Copy link

mra-dev commented Apr 3, 2024

I actually found a nice and easy workaround. Just use Laravel 10's default EventServiceProvider & add it to bootstrap/providers.php's Array & set shouldDiscoverEvents to return true. And suddenly everything's just working fine.

// app/Providers/EventServiceProvider.php

<?php

namespace App\Providers;

use Illuminate\Foundation\Support\Providers\EventServiceProvider as ServiceProvider;

class EventServiceProvider extends ServiceProvider
{
    public function shouldDiscoverEvents(): bool
    {
        return true;
    }
}
// bootstrap/providers.php

<?php

use App\Providers\EventServiceProvider;

return [
    App\Providers\AppServiceProvider::class,
    EventServiceProvider::class,
];

running artisan command:
php artisan event:list

before:

...
...
  Illuminate\Queue\Events\JobProcessed .............................................................................................................
  ⇂ Closure at: /vendor/spatie/laravel-ignition/src/IgnitionServiceProvider.php:263
  Illuminate\Queue\Events\JobProcessing ............................................................................................................
  ⇂ Closure at: /vendor/laravel/framework/src/Illuminate/Log/Context/ContextServiceProvider.php:38
  ⇂ Closure at: /vendor/spatie/laravel-ignition/src/IgnitionServiceProvider.php:257

after:

...
...
  Illuminate\Queue\Events\JobProcessed .............................................................................................................
  ⇂ Closure at: /vendor/spatie/laravel-ignition/src/IgnitionServiceProvider.php:263
  Illuminate\Queue\Events\JobProcessing ............................................................................................................
  ⇂ Closure at: /vendor/laravel/framework/src/Illuminate/Log/Context/ContextServiceProvider.php:38
  ⇂ Closure at: /vendor/spatie/laravel-ignition/src/IgnitionServiceProvider.php:257

  MraDev\Product\Events\UploadMedia ................................................................................................................
  ⇂ MraDev\Upload\Listeners\ProcessUploadingMedia@handle

@inxilpro
Copy link
Contributor

inxilpro commented Apr 3, 2024

Unfortunately this is looking like a harder thing the fix than I hoped. We can’t extend the Laravel service provider any more, so we need to replace more than I expected. I have it partially fixed, but I’m on spring break with my kids, so I have less time to work on it than usual. I’m gonna be on a train for 5 hours today, so hopefully I can get this sorted out then.

@inxilpro
Copy link
Contributor

inxilpro commented Apr 3, 2024

@mra-dev can you try #88 and see if that works for you? Pretty sure that should fix it.

@mra-dev
Copy link

mra-dev commented Apr 3, 2024

@inxilpro Yeah for sure. How should i do that? Update it from composer.json or ...?

@mra-dev
Copy link

mra-dev commented Apr 3, 2024

@inxilpro I almost forgot. I tested it manually by editing vendor files and dump-autoload composer and then got the event:list & it totally worked fine and listed my newly added Events/Listeners.

Then i thought maybe i should manually turn Autodiscovery ON with Laravel's EventServiceProvider & add it to providers (Taylor said in his conference video that you can already use old providers if you wish to extend). And it worked like a charm. 😍

Like This: Solution

@inxilpro
Copy link
Contributor

inxilpro commented Apr 3, 2024

@mra-dev you can do composer require internachi/modular:dev-laravel-11-event-support to install it from that branch and try it out.

@mra-dev
Copy link

mra-dev commented Apr 3, 2024

@inxilpro Did you check my Solution yourself & what do you think of it ?

@inxilpro
Copy link
Contributor

inxilpro commented Apr 3, 2024

@inxilpro Did you check my Solution yourself & what do you think of it ?

That solution is great! I just want to make sure that the package works without any workarounds…

@inxilpro inxilpro closed this as completed Apr 3, 2024
@inxilpro
Copy link
Contributor

inxilpro commented Apr 3, 2024

(I'm going to close this because I just confirmed that #88 works in Laravel 11.)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants