Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[5.2] Fire the RouteMatched event on route:list command #13474

Merged
merged 1 commit into from
May 18, 2016
Merged

[5.2] Fire the RouteMatched event on route:list command #13474

merged 1 commit into from
May 18, 2016

Conversation

themsaid
Copy link
Member

@themsaid themsaid commented May 7, 2016

As described here #13453, FormRequests wouldn't get initialised properly when being resolved from the routes:list command, this is because the initialisation logic only runs when the RouteMatched event is fired.

Check:

$this->app['events']->listen(RouteMatched::class, function () {

@taylorotwell
Copy link
Member

But we're not passing anything to the event. Wouldn't any listener thats trying to access the event's data fail?

@themsaid
Copy link
Member Author

themsaid commented May 9, 2016

Yes, you're right, it just seemed that no listener is using the event attributes in the core that's why I didn't notice there are any in the event class constructor.

What do you think if we remove the event listener from the provider and just use $this->app->resolving? Here it doesn't need to know about the route or the request (Event Parameters), so I assume it should be ok, no?

$this->app->resolving(function (FormRequest $request, $app) {
    $this->initializeRequest($request, $app['request']);

    $request->setContainer($app)->setRedirector($app->make(Redirector::class));
});

Or should we fire the event for every route inside the foreach loop of the command?

foreach ($this->routes as $route)

@taylorotwell
Copy link
Member

You can try changing to resolving. I'm not sure what effects that may have.

… FormRequests

        initisalize FormRequest regardless of the RouteMatched event
@themsaid
Copy link
Member Author

@taylorotwell changed the code so that the resolving call back is registered by default regardless of whether the RouteMatched event is fired or not.

@taylorotwell
Copy link
Member

I wonder why I had it only registering if Route Matched before? :/

@themsaid
Copy link
Member Author

Yeah I was confused about that, but thought maybe there's some reason I'm not aware of.

@taylorotwell taylorotwell merged commit b5ffdb1 into laravel:5.2 May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants