Framework: event:generate command namespace issue

Created on 9 Feb 2019  路  13Comments  路  Source: laravel/framework

  • Laravel Version: 5.7.25 || 5.8.2
  • PHP Version: 7.2.13
  • Database Driver & Version: NA

Description:

php artisan event:generate command assumes that event class exists in app\Events

Steps To Reproduce:

Update your EventServiceProvider with event class that is coming from vendor, for example

protected $listen = [
    'Laravel\Passport\Events\AccessTokenCreated' => [
        'App\Listeners\RevokeOldTokens',
    ],
];

Run php artisan event:generate
Check your app/Listeners/PruneOldTokens.php file, it should have wrong namespace imported.

use App\Events\Laravel\Passport\Events\RefreshTokenCreated;
bug

Most helpful comment

28007 was merged.

Note: Using event:generate as described by the OP, will still not work.

The command expects one of three event namespace styles:

  1. Your event is in your local project's namespace (eg. App)

    • In this case, either import your event at the top of the service provider, or if it doesn't exist yet, just use the string version(eg. 'App\Events\MyEvent').

  2. Your event is in the Illuminate namespace.

    • You can either import the event class or use the string version starting with 'Illuminate'.

  3. Your event is in a third-party namespace.

    • You must use the string version and start it with a '\'. (eg. '\MyGreatVendor\MyGreatPackage\MyGreatEvent')

I guess this could be looked at in the future for a better way to handle all of these cases without having to differentiate, but for now, this should work.

All 13 comments

Seems like a job for the make:listener command.

What happens when you use the ::class notation instead?

Tried this

    protected $listen = [
        \Laravel\Passport\Events\AccessTokenCreated::class => [
            'App\Listeners\RevokeOldTokens',
        ],
    ];

But same results

I can not reproduce it on laravel 5.7.x

That only will happen if the event class does not exist.
I think you have some typos or something in the namespace of the event class.

@imanghafoori1
I have laravel passport installed, so i am sure that
Laravel\Passport\Events\AccessTokenCreated
event class exists

I can not reproduce the bug on 5.7.x It does successfully import like the following

use App\Providers\Laravel\Passport\Events\AccessTokenCreated;

But, if you don't install passport it seems like the behavior you described. Please make sure for us.

5.7.28

'Foo\Bar\Nonexisting\Event' => [
    'App\Listeners\Foo',
],
$ cat Events/Foo/Bar/Nonexisting/Event.php
<?php

namespace App\Events\Foo\Bar\Nonexisting;

I think the correct behavior in the case of nonexisting non-\App events would be throwing an error?

Another finding -
The command work well with event from Illuminate namespace

// EventServiceProvider
protected $listen = [
    'Illuminate\Notifications\Events\NotificationSent' => [
        'App\Listeners\LogNotification',
    ],
];

I've looked over the code involved with event:generate and believe I've come up with the problem and a solution. Looking for feedback before making a PR.

It all happens in the ListenerMakeCommand class.

There is a check to see if the given event namespace starts with (1)your project namespace(App), (2)"Illuminate", or (3)"\".

So it looks like the command expects 3 situations:

  1. Your event is somewhere in your project's namespace.
  2. Your event is in the Illuminate namespace.
  3. Your event is elsewhere and you are showing that by starting with a '\'.

If those assumptions are true, then you should be able to generate listeners from events in third party namespaces by adding a starting '\' to your event in the $listen array of your EventServiceProvider.
However, if you do that, then the '\' doesn't get dropped and the namespace at the top of the generated listener for your event, will still have the starting '\'.

I propose a simple call to trim when replacing the 'DummyFullEvent' at the top of the listener class.
So instead of:

    return str_replace(
        'DummyFullEvent', $event, $stub
    );

We can do:

    return str_replace(
        'DummyFullEvent', trim($event, '\\'), $stub
    );

And the Event namespace will be correct in the Listener class.

Thoughts?

I've ran several test scenarios with this change in place and everything seems to work as expected?

@devcircus lgtm 馃憤

PR was merged.

28007 was merged.

Note: Using event:generate as described by the OP, will still not work.

The command expects one of three event namespace styles:

  1. Your event is in your local project's namespace (eg. App)

    • In this case, either import your event at the top of the service provider, or if it doesn't exist yet, just use the string version(eg. 'App\Events\MyEvent').

  2. Your event is in the Illuminate namespace.

    • You can either import the event class or use the string version starting with 'Illuminate'.

  3. Your event is in a third-party namespace.

    • You must use the string version and start it with a '\'. (eg. '\MyGreatVendor\MyGreatPackage\MyGreatEvent')

I guess this could be looked at in the future for a better way to handle all of these cases without having to differentiate, but for now, this should work.

30796 I'v open a new issure for my problem.

Three ways only one could be work, and the others would not.
I do not think the problems were solved.

Was this page helpful?
0 / 5 - 0 ratings