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.4] Allow non-method Callables #19168

Merged
merged 2 commits into from
May 12, 2017
Merged

[5.4] Allow non-method Callables #19168

merged 2 commits into from
May 12, 2017

Conversation

ConnorVG
Copy link
Contributor

Currently if you have a dynamic controller method (via __call) it will fail because of ReflectionMethod not finding the explicit method.

Here is an example, say we have ViewController that is simply:

namespace App\Http\Controllers;

class ViewController extends Controller
{
    /**
     * Get a static page view.
     *
     * @param string $method
     * @param array  $arguments
     *
     * @return \Illuminate\Http\Response
     */
    public function __call($method, $arguments = [])
    {
        return view($method)
            ->with($arguments);
    }
}

A route definition of:

$router->get('/', [
    'as' => 'get::home',
    'uses' => 'ViewController@home',
]);

and a view home.blade.php of:

<h1>We're home!</h1>

With the current setup it will fail with Method App\Http\Controllers\ViewController::home() does not exist but with the alteration it will succeed as it is still callable just not an explicit method.

@m1guelpf
Copy link
Contributor

Could you add some tests for this?

@ntzm
Copy link
Contributor

ntzm commented May 12, 2017

I think this is a good idea 👍

@ConnorVG ConnorVG changed the title Allow non-method Callables [5.4] Allow non-method Callables May 12, 2017
@taylorotwell taylorotwell merged commit 0cf1f76 into laravel:5.4 May 12, 2017
@ConnorVG ConnorVG deleted the patch-2 branch May 12, 2017 13:00
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.

4 participants