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

In 4.0 make Router fluent #10643

Open
lukewatts opened this Issue May 11, 2017 · 30 comments

Comments

Projects
None yet
8 participants
@lukewatts

lukewatts commented May 11, 2017

I think v4.0 would greatly benefit from a fluent interface for routing. So instead of the current Route with array:

Router::scope('/', function (RouteBuilder $routes) {
    $routes->connect('/', ['controller' => 'Pages', 'action' => 'display', 'home']);
}

Fluent (chained) routes:

use App\Controller\Pages;

Router::scope('/', function (RouteBuilder $routes) {
    $routes->connect('/', 'Pages::display')->view('home');
}

Benefits include:

  • Better IDE support and auto-completion via IntelliSense.
  • Less typing. Not so WET (We Enjoy Typing)
  • More inline with conventions in every other large PHP MVC framework (Laravel/Lumen, Slim, Silex all have fluent Routes)
  • It's just cooler :)
  • enhancement

  • feature-discussion (RFC)

  • CakePHP Version:4.0.

@cleptric cleptric added this to the 4.0.0 milestone May 11, 2017

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 11, 2017

Member

What would the view() method in the second example do?

As part of #10308 I'm going to be adding a routes() hook to the Application and deprecating all of the static route building methods on Router. This idea and that work together nicely, but I'd like to understand what fluent features you'd like to see.

Member

markstory commented May 11, 2017

What would the view() method in the second example do?

As part of #10308 I'm going to be adding a routes() hook to the Application and deprecating all of the static route building methods on Router. This idea and that work together nicely, but I'd like to understand what fluent features you'd like to see.

@lukewatts

This comment has been minimized.

Show comment
Hide comment
@lukewatts

lukewatts May 11, 2017

@markstory The view() should actually be name() for the named routes.

I mainly just want to see less arrays wherever possible in routing.

There could also be an optional param in connect for anything which has to be in an array (or to allow for gradual migration of options to chained methods), such as:

$routes->connect('/', 'Pages::display', ['something' => 'here'])->name('home');

@markstory The view() should actually be name() for the named routes.

I mainly just want to see less arrays wherever possible in routing.

There could also be an optional param in connect for anything which has to be in an array (or to allow for gradual migration of options to chained methods), such as:

$routes->connect('/', 'Pages::display', ['something' => 'here'])->name('home');
@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 11, 2017

Member

The Pages::display DSL needs some more thinking. A few questions that come me are:

  • How do prefixes and plugins work into it?
  • What about passed arguments, or application level keys?
  • How if at all does this DSL get used when making URLs?
Member

markstory commented May 11, 2017

The Pages::display DSL needs some more thinking. A few questions that come me are:

  • How do prefixes and plugins work into it?
  • What about passed arguments, or application level keys?
  • How if at all does this DSL get used when making URLs?
@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 12, 2017

I also think the route arguments are too messy

slince commented May 12, 2017

I also think the route arguments are too messy

@jrbasso

This comment has been minimized.

Show comment
Hide comment
@jrbasso

jrbasso May 12, 2017

Member

An option to eliminate the arrays and avoiding the DSL (which I'm not a big fan, specially when the features grow that start to becoming very hack and you need almost a compiler to parse it) is using a PHPUnit style. Something like:

$route->for('/')
  ->useControllerAction('Pages', 'display')
  ->withName('home');

You can also use the same for specializing:

$route->for('/magic')
  ->requestMethod(Request::METHOD_GET)
  ->useControllerAction('App\CustomDomain\Controller\Homepage', 'display')
  ->usingPlugin('Abc')
  ->addRequestParam('specialKey', 'value');

The useControllerAction can be other things like useCallback or redirect, etc.

Member

jrbasso commented May 12, 2017

An option to eliminate the arrays and avoiding the DSL (which I'm not a big fan, specially when the features grow that start to becoming very hack and you need almost a compiler to parse it) is using a PHPUnit style. Something like:

$route->for('/')
  ->useControllerAction('Pages', 'display')
  ->withName('home');

You can also use the same for specializing:

$route->for('/magic')
  ->requestMethod(Request::METHOD_GET)
  ->useControllerAction('App\CustomDomain\Controller\Homepage', 'display')
  ->usingPlugin('Abc')
  ->addRequestParam('specialKey', 'value');

The useControllerAction can be other things like useCallback or redirect, etc.

@lukewatts

This comment has been minimized.

Show comment
Hide comment
@lukewatts

lukewatts May 12, 2017

That's even more verbose than the arrays. And the above example doesn't allow for callbacks in place of the controller, which was what I was thinking of.

Pages::display isn't really a DSL. It's just <controller>::<method>.

You just need to use explode by the separator (currently :: but could be anything...Laravel uses '@').

Then you can map it to the controller and action with the variables in whatever method.

So with the syntax by @jrbasso in mind and readability of "For x I do x with x and with x":

$route->for('/)->doAction('Pages::display', ['params' => 'passed to view'])->withMethod(Request::METHOD_GET)->withPlugin(Abc');

Then I think we could abstract that away to method specific flows:

$route->get('/', 'Pages::show', ['optional' => 'params'])->usePlugin('Abc');
$route->post('/', 'Pages::save', ['optional' => 'params'])->usePlugin('Abc');
$route->put('/', 'Controller::update', ['optional' => 'params'])->usePlugin('Abc');
$route->delete('/', 'Controller::delete', ['optional' => 'params'])->usePlugin('Abc');
$route->head('/', 'Controller::head', ['optional' => 'params'])->usePlugin('Abc');
$route->options('/', 'Controller::options', ['optional' => 'params'])->usePlugin('Abc');`

That way it's very clear what is happening from looking at the routes file.

Thee current routes file is confusing for people new to CakePHP.

Point is each option/use case can be broken out into a method replace the arrays for the first implementation and then wrappers/facades can be made to make it easier to read and type for the 90% of use cases. Then anyone in that 10% who needs full control has the core (more verbose) implementation.

lukewatts commented May 12, 2017

That's even more verbose than the arrays. And the above example doesn't allow for callbacks in place of the controller, which was what I was thinking of.

Pages::display isn't really a DSL. It's just <controller>::<method>.

You just need to use explode by the separator (currently :: but could be anything...Laravel uses '@').

Then you can map it to the controller and action with the variables in whatever method.

So with the syntax by @jrbasso in mind and readability of "For x I do x with x and with x":

$route->for('/)->doAction('Pages::display', ['params' => 'passed to view'])->withMethod(Request::METHOD_GET)->withPlugin(Abc');

Then I think we could abstract that away to method specific flows:

$route->get('/', 'Pages::show', ['optional' => 'params'])->usePlugin('Abc');
$route->post('/', 'Pages::save', ['optional' => 'params'])->usePlugin('Abc');
$route->put('/', 'Controller::update', ['optional' => 'params'])->usePlugin('Abc');
$route->delete('/', 'Controller::delete', ['optional' => 'params'])->usePlugin('Abc');
$route->head('/', 'Controller::head', ['optional' => 'params'])->usePlugin('Abc');
$route->options('/', 'Controller::options', ['optional' => 'params'])->usePlugin('Abc');`

That way it's very clear what is happening from looking at the routes file.

Thee current routes file is confusing for people new to CakePHP.

Point is each option/use case can be broken out into a method replace the arrays for the first implementation and then wrappers/facades can be made to make it easier to read and type for the 90% of use cases. Then anyone in that 10% who needs full control has the core (more verbose) implementation.

@inoas

This comment has been minimized.

Show comment
Hide comment
@inoas

inoas May 12, 2017

Contributor

In general I think a reason to make an interface fluid is that you can throw errors when methods do not exist, where as for array keys you/we do not (as easy).

So IMHO if there is a desire to do this, the focus should be to make creation of routes less error prone and more informative if errors occur.

If that cannot be done, I do personally see little reason to change the interface.

Contributor

inoas commented May 12, 2017

In general I think a reason to make an interface fluid is that you can throw errors when methods do not exist, where as for array keys you/we do not (as easy).

So IMHO if there is a desire to do this, the focus should be to make creation of routes less error prone and more informative if errors occur.

If that cannot be done, I do personally see little reason to change the interface.

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 12, 2017

Member

@lukewatts If the Pages::display syntax is only used as a shortcut when defining the controller/action then we would rely on the fluent methods to set the other attributes like prefixes, plugin and app level attributes.

I like the get/put/post methods on the builder and that is something we can add well before 4.0.

Member

markstory commented May 12, 2017

@lukewatts If the Pages::display syntax is only used as a shortcut when defining the controller/action then we would rely on the fluent methods to set the other attributes like prefixes, plugin and app level attributes.

I like the get/put/post methods on the builder and that is something we can add well before 4.0.

@lukewatts

This comment has been minimized.

Show comment
Hide comment
@lukewatts

lukewatts May 13, 2017

@markstory I would love to see the get,put, post methods implemented. Those are probably the most important thing in my opinion. That way I just know what the purpose of that route is by glancing at the routes file.

Regarding the Pages::display syntax it's just a convention I've seen in many PHP framework I'm familiar with (Laravel, Silex, Slim, ). If it's a problem I'm sure another solution will work. Also, generally you can also then use any callable (in place of a controller::action) and define smaller functionality directly in the route. A trivial example where we append Hello {$name} to the Request body:

$route->get('/hello/{name}', function($name) {
    // Modify the $request headers here if need be.
    $output = 'Hello '. $name;

    $this->request->body($output . $this->getBody());

    return $this->request->getBody();
});

lukewatts commented May 13, 2017

@markstory I would love to see the get,put, post methods implemented. Those are probably the most important thing in my opinion. That way I just know what the purpose of that route is by glancing at the routes file.

Regarding the Pages::display syntax it's just a convention I've seen in many PHP framework I'm familiar with (Laravel, Silex, Slim, ). If it's a problem I'm sure another solution will work. Also, generally you can also then use any callable (in place of a controller::action) and define smaller functionality directly in the route. A trivial example where we append Hello {$name} to the Request body:

$route->get('/hello/{name}', function($name) {
    // Modify the $request headers here if need be.
    $output = 'Hello '. $name;

    $this->request->body($output . $this->getBody());

    return $this->request->getBody();
});
@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 13, 2017

Member

Does anyone want to propose the set of methods we would need for the fluent setters?

Member

markstory commented May 13, 2017

Does anyone want to propose the set of methods we would need for the fluent setters?

@inoas

This comment has been minimized.

Show comment
Hide comment
@inoas

inoas May 14, 2017

Contributor

I do agree with the http method verbs being placeable in a prominent place in the router. The router's interface could be much more like Sinatra / Slim / Silex / Lumen.

Contributor

inoas commented May 14, 2017

I do agree with the http method verbs being placeable in a prominent place in the router. The router's interface could be much more like Sinatra / Slim / Silex / Lumen.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 14, 2017

@markstory I agree with @lukewatts , the idea of is very good; Except, defects on the use of the ":" placeholder should also be changed.

slince commented May 14, 2017

@markstory I agree with @lukewatts , the idea of is very good; Except, defects on the use of the ":" placeholder should also be changed.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 14, 2017

And automatically inject routing parameters for action; removes "pass" option

slince commented May 14, 2017

And automatically inject routing parameters for action; removes "pass" option

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 14, 2017

Member

@slince Couldn't the pass option be part of the fluent methods that would be added?

Changing :key to another format would be harder to make backwards compatible as we would need to support both the new and old placeholder formats.

Member

markstory commented May 14, 2017

@slince Couldn't the pass option be part of the fluent methods that would be added?

Changing :key to another format would be harder to make backwards compatible as we would need to support both the new and old placeholder formats.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 14, 2017

we would need to support both the new and old placeholder formats.

Yeah, the developers can choose ":placeholder" or "{placeholder}" now ; Gradually abandoned later ":"

Couldn't the pass option be part of the fluent methods that would be added?

The route parameters can be detected by reflection, it is rellay not necessary to confirm that by pass option;

       // defines a action
       $action = function($name, $age) {
            echo "The user [{$name}] age is {$age} years old";
       }

       // creats a route to this action( only two necessary arguments; so clear)
       $routeBuilder->create('/users/{name}/{age}/{gender}',  $action )->method(['get', 'post'])
           ->host('{subdomain}.domain.com') //set host
           ->name('user.info')  // set name
           ->defaults([   // give default values for placeholder if the placeholder is not necessary
                 'name' => 'Justin Bieber',
                 'age' => 18,
                 'subdomain' => 'profile'
            ]);

        // if the route is matched, find the necessary parameters for the action from array of "placeholder"
        $definedPaameters = getActionParameters($action);
        $filteredParameters = array_intersect_key($placeholderParameters, $definedPaameters);

        if  (count($filteredParameters ) < count($definedPaameters ))  {
             throw new RuntimeException("Missing parameters");
        }

        function getActionParameters($action)
        {
            $reflection = new \ReflectionFunction($action);
            $arguments = $reflection->getParameters();
            return array_map(function($argument){
                return $argument->name;
            }, $arguments);
        }


        //finally, call action
         call_user_func_array($action, $filteredParameters );

slince commented May 14, 2017

we would need to support both the new and old placeholder formats.

Yeah, the developers can choose ":placeholder" or "{placeholder}" now ; Gradually abandoned later ":"

Couldn't the pass option be part of the fluent methods that would be added?

The route parameters can be detected by reflection, it is rellay not necessary to confirm that by pass option;

       // defines a action
       $action = function($name, $age) {
            echo "The user [{$name}] age is {$age} years old";
       }

       // creats a route to this action( only two necessary arguments; so clear)
       $routeBuilder->create('/users/{name}/{age}/{gender}',  $action )->method(['get', 'post'])
           ->host('{subdomain}.domain.com') //set host
           ->name('user.info')  // set name
           ->defaults([   // give default values for placeholder if the placeholder is not necessary
                 'name' => 'Justin Bieber',
                 'age' => 18,
                 'subdomain' => 'profile'
            ]);

        // if the route is matched, find the necessary parameters for the action from array of "placeholder"
        $definedPaameters = getActionParameters($action);
        $filteredParameters = array_intersect_key($placeholderParameters, $definedPaameters);

        if  (count($filteredParameters ) < count($definedPaameters ))  {
             throw new RuntimeException("Missing parameters");
        }

        function getActionParameters($action)
        {
            $reflection = new \ReflectionFunction($action);
            $arguments = $reflection->getParameters();
            return array_map(function($argument){
                return $argument->name;
            }, $arguments);
        }


        //finally, call action
         call_user_func_array($action, $filteredParameters );
@slince

This comment has been minimized.

Show comment
Hide comment
@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 14, 2017

Member

That could work well, we'd need to account for parameter name inflections and parameter re-ordering, but the concept is reasonable. I'm not sure route defaults will work out well in practice.

The example you provided with defaults could result in a route that overlaps with other routes. e.g. /users/:name and /users/:name/:age could also be routes, and the defaults make matching more ambiguous.

Member

markstory commented May 14, 2017

That could work well, we'd need to account for parameter name inflections and parameter re-ordering, but the concept is reasonable. I'm not sure route defaults will work out well in practice.

The example you provided with defaults could result in a route that overlaps with other routes. e.g. /users/:name and /users/:name/:age could also be routes, and the defaults make matching more ambiguous.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 14, 2017

The example you provided with defaults could result in a route that overlaps with other routes

That is not a problem,

  $routeBuilder->create('/users/{name}/{age}',  $action1)->name('route1')->defaults([
      'age' => 18
  ]);
  $routeBuilder->create('/users/{name}',  $action2)->name('route2');

Because the age of route1 has default value , in theory, the url /users/justin can match both routes; however; the route1 defines first; so route1 is matched finally;

except. the developers will avoid to create ambiguous routes; If the two routes are very similar, then they should be merged into one;

The default parameter support allows the developer to write fewer routes and keep "DRY"

slince commented May 14, 2017

The example you provided with defaults could result in a route that overlaps with other routes

That is not a problem,

  $routeBuilder->create('/users/{name}/{age}',  $action1)->name('route1')->defaults([
      'age' => 18
  ]);
  $routeBuilder->create('/users/{name}',  $action2)->name('route2');

Because the age of route1 has default value , in theory, the url /users/justin can match both routes; however; the route1 defines first; so route1 is matched finally;

except. the developers will avoid to create ambiguous routes; If the two routes are very similar, then they should be merged into one;

The default parameter support allows the developer to write fewer routes and keep "DRY"

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 14, 2017

Added, the route option also need to support scheme

       $routeBuilder->create('/users/{name}/{age}/{gender}',  $action )
           ->method(['get', 'post'])
           ->host('{subdomain}.domain.com') //set host

           ->scheme(['http', 'https']);   //set http scheme

           ->name('user.info')  // set name
           ->defaults([   // give default values for placeholder if the placeholder is not necessary
                 'name' => 'Justin Bieber',
                 'age' => 18,
                 'subdomain' => 'profile'
            ]);

slince commented May 14, 2017

Added, the route option also need to support scheme

       $routeBuilder->create('/users/{name}/{age}/{gender}',  $action )
           ->method(['get', 'post'])
           ->host('{subdomain}.domain.com') //set host

           ->scheme(['http', 'https']);   //set http scheme

           ->name('user.info')  // set name
           ->defaults([   // give default values for placeholder if the placeholder is not necessary
                 'name' => 'Justin Bieber',
                 'age' => 18,
                 'subdomain' => 'profile'
            ]);

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 14, 2017

Member

Couldn't default arguments be defined in the controller action? I don't know if the added complexity and risk of ambiguity is a good idea when actions could have default arguments. The other methods sound good.

Member

markstory commented May 14, 2017

Couldn't default arguments be defined in the controller action? I don't know if the added complexity and risk of ambiguity is a good idea when actions could have default arguments. The other methods sound good.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 15, 2017

@markstory What do you mean like that?

  $routeBuilder->create('/users/{name}',  function($name = 'Justin'){
        //...
  });

However,this route does not match /users and can only match/users/justin;

we need to tell route matcher that the placeholder is optional. The default option is for this purpose.

slince commented May 15, 2017

@markstory What do you mean like that?

  $routeBuilder->create('/users/{name}',  function($name = 'Justin'){
        //...
  });

However,this route does not match /users and can only match/users/justin;

we need to tell route matcher that the placeholder is optional. The default option is for this purpose.

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 15, 2017

Member

I meant that if a controller action has optional parameters and multiple routes that resolve to it, default arguments in the function would be simpler overall.

// Routes
$routes->get('/users/{name}', 'Users::getPerson');
$routes->get('/users/{name}/{thumbs}', 'Users::getPerson');

// action 
function getPerson($name, $age = 21, $thumbs = 2) {
}

This approach would avoid the ambiguity that the defaults() approach introduces, at the cost of requiring additional routes to be declared. I can see how the additional route is not as ideal, but I'm worried about expanding the complexity and runtime cost of routing.

Member

markstory commented May 15, 2017

I meant that if a controller action has optional parameters and multiple routes that resolve to it, default arguments in the function would be simpler overall.

// Routes
$routes->get('/users/{name}', 'Users::getPerson');
$routes->get('/users/{name}/{thumbs}', 'Users::getPerson');

// action 
function getPerson($name, $age = 21, $thumbs = 2) {
}

This approach would avoid the ambiguity that the defaults() approach introduces, at the cost of requiring additional routes to be declared. I can see how the additional route is not as ideal, but I'm worried about expanding the complexity and runtime cost of routing.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 15, 2017

@markstory
I have unstandard what you meant;

  1. But this will cause that need create many routes for one action; The more routes will lead to worse performance. Is not it?

An example:

$routes->get('/products', 'Products::index');
$routes->get('/products/{page}', 'Products::index');
// action 
public function index($page = 1) {
}

These two routes are doing the same thing; Is it really necessary to create two routes for an action?

  1. Some placeholders maybe belong to host pattern not path pattern ;
  • How to provide default value for placeholder in controller action?
  • If you want to ommit the placeholders that have default value, how to generate url for the route?
$routes->get('/users/{name}', 'Users::getPerson')->host('{subdomain}.domain.com')

However if the route has defaults, you can ommit that;

 $route = $routeBuilder->create('/users/{name}',  'Users::getPerson')
        ->host('{subdomain}.domain.com') //set host
        ->scheme('https');
        ->defaults([
             'subdomian' => 'profile'
        ]);

$routeGenerator->generate($route, ['name' => 'Bob']);  //output "/users/bob"

//use the default value; so easy
$routeGenerator->generate($route, ['name' => 'Bob'], true);  //output "https://profile.domain.com/users/bob"

//use a custom value
$routeGenerator->generate($route, ['name' => 'Bob', 'subdomain' => 'user'], true);  //output "https://user.domain.com/users/bob"
  1. Optional placeholder is very helpful and has been implement by other routing package; some links:
    Auraphp router
    Laravel routing

But I do not like laravel's solution.I love symfony's [routing]
(https://symfony.com/doc/current/routing/optional_placeholders.html)

slince commented May 15, 2017

@markstory
I have unstandard what you meant;

  1. But this will cause that need create many routes for one action; The more routes will lead to worse performance. Is not it?

An example:

$routes->get('/products', 'Products::index');
$routes->get('/products/{page}', 'Products::index');
// action 
public function index($page = 1) {
}

These two routes are doing the same thing; Is it really necessary to create two routes for an action?

  1. Some placeholders maybe belong to host pattern not path pattern ;
  • How to provide default value for placeholder in controller action?
  • If you want to ommit the placeholders that have default value, how to generate url for the route?
$routes->get('/users/{name}', 'Users::getPerson')->host('{subdomain}.domain.com')

However if the route has defaults, you can ommit that;

 $route = $routeBuilder->create('/users/{name}',  'Users::getPerson')
        ->host('{subdomain}.domain.com') //set host
        ->scheme('https');
        ->defaults([
             'subdomian' => 'profile'
        ]);

$routeGenerator->generate($route, ['name' => 'Bob']);  //output "/users/bob"

//use the default value; so easy
$routeGenerator->generate($route, ['name' => 'Bob'], true);  //output "https://profile.domain.com/users/bob"

//use a custom value
$routeGenerator->generate($route, ['name' => 'Bob', 'subdomain' => 'user'], true);  //output "https://user.domain.com/users/bob"
  1. Optional placeholder is very helpful and has been implement by other routing package; some links:
    Auraphp router
    Laravel routing

But I do not like laravel's solution.I love symfony's [routing]
(https://symfony.com/doc/current/routing/optional_placeholders.html)

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 16, 2017

Member

Yes, more routes does increase route match time. The use cases you illustrated do show how default values can be useful in more than just positional argument scenarios. I think we would have to be careful when adding them to ensure performance doesn't degrade.

From the thread above it sounds like there are a few different but related improvements people want to see done for route definition. First there are a set of changes intended to make the ergonomics of defining routes simpler:

  • Addition of HTTP method specific shorthands $routes->post(), $routes->put() etc.
  • Compact defintion of Articles::view pairs. This is just a sugared version of ['controller' => 'Articles', 'action' => 'view']. It has no option/features for specifying more than the controller/action.
  • Fluent interface methods for existing 'options'. The existing options that could have fluent setters defined are name(), method(), host(), requirements()

In addition, there are entirely new features that would expand the feature set of routing:

  • New placeholder format {name} instead of :name. This new format has fewer issues with mid-word placeholders being incorrectly interpreted. See #9188 for an example of a route that cannot be created with today's :var format.
  • Scheme matching.
  • Deprecation of the pass option. Instead controller pass arguments would be name matched based on parameter names.
  • Placeholders in host conditions.
  • Route default values. Optional values that fill out placeholders if they are missing.
  • Callable binding. Instead of a controller/action, routes could have callables provided, allowing a more 'microframework' style usage.

I'm not sure all off these can be implemented in the upcoming 3.5 release, but we can make issues for them and get them implemented as people make time to see each feature through.

This is a pretty dense thread, so let me know if I've missed anything.

edits

  • Added requirements method.
Member

markstory commented May 16, 2017

Yes, more routes does increase route match time. The use cases you illustrated do show how default values can be useful in more than just positional argument scenarios. I think we would have to be careful when adding them to ensure performance doesn't degrade.

From the thread above it sounds like there are a few different but related improvements people want to see done for route definition. First there are a set of changes intended to make the ergonomics of defining routes simpler:

  • Addition of HTTP method specific shorthands $routes->post(), $routes->put() etc.
  • Compact defintion of Articles::view pairs. This is just a sugared version of ['controller' => 'Articles', 'action' => 'view']. It has no option/features for specifying more than the controller/action.
  • Fluent interface methods for existing 'options'. The existing options that could have fluent setters defined are name(), method(), host(), requirements()

In addition, there are entirely new features that would expand the feature set of routing:

  • New placeholder format {name} instead of :name. This new format has fewer issues with mid-word placeholders being incorrectly interpreted. See #9188 for an example of a route that cannot be created with today's :var format.
  • Scheme matching.
  • Deprecation of the pass option. Instead controller pass arguments would be name matched based on parameter names.
  • Placeholders in host conditions.
  • Route default values. Optional values that fill out placeholders if they are missing.
  • Callable binding. Instead of a controller/action, routes could have callables provided, allowing a more 'microframework' style usage.

I'm not sure all off these can be implemented in the upcoming 3.5 release, but we can make issues for them and get them implemented as people make time to see each feature through.

This is a pretty dense thread, so let me know if I've missed anything.

edits

  • Added requirements method.
@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 16, 2017

@markstory that sounds great;

Maybe requirements should be filled;

$route = $builder->create('/users/{name}',  'Users::getPerson')
        ->name('user.profile')
        ->method(['get', 'post'])
        ->host('{subdomain}.domain.com') //set host
        ->scheme('https')

        ->requirements([
               'name' => '\w+',
               'subdomain' => 'profile|user'
        ])

        ->defaults([
             'subdomian' => 'profile'
        ]);

Grammar sugar for create

$builder->get/post/put/head/delete/options/any

Route generator should contain at least 3 methods

//Generate url for the route
$url = $generator->generate($route,  ['name' => 'foo', 'hello' => 'world']); // output  "/users/foo?hello=world"

//find the route by its action and generate url for it
$url = $generator->generateToAction('Users::getPerson', ['name' => 'foo']);

//find the route by a name and generate url for it
$url = $generator->generateByName('user.profile',  ['name' => 'foo']);

Note: becauseof my poor english, the method name may not be appropriate.

Subcollection of routes

$builder->group([
    'host' => 'profile.domain.com', 
    'scheme' => 'https', 
    'middleware'=>['Auth', 'Api']
], function(RouteBuilder $routes) {
     $routes->get/post/put.../any
})

Group supports the following option

  • host
  • method
  • scheme
  • requirements
  • defaults
  • prefix

And other common parameters that need to be stored for some custom services like:

  • middleware
  • plugin

Custom service should not be provided by routing package. route just store the paramters.

class Route
{
   protected $path;
   protected $action;
   protected $name;
   protected $method;
   //...
   protected $parameters; //Stores custom paramters for other services
}

slince commented May 16, 2017

@markstory that sounds great;

Maybe requirements should be filled;

$route = $builder->create('/users/{name}',  'Users::getPerson')
        ->name('user.profile')
        ->method(['get', 'post'])
        ->host('{subdomain}.domain.com') //set host
        ->scheme('https')

        ->requirements([
               'name' => '\w+',
               'subdomain' => 'profile|user'
        ])

        ->defaults([
             'subdomian' => 'profile'
        ]);

Grammar sugar for create

$builder->get/post/put/head/delete/options/any

Route generator should contain at least 3 methods

//Generate url for the route
$url = $generator->generate($route,  ['name' => 'foo', 'hello' => 'world']); // output  "/users/foo?hello=world"

//find the route by its action and generate url for it
$url = $generator->generateToAction('Users::getPerson', ['name' => 'foo']);

//find the route by a name and generate url for it
$url = $generator->generateByName('user.profile',  ['name' => 'foo']);

Note: becauseof my poor english, the method name may not be appropriate.

Subcollection of routes

$builder->group([
    'host' => 'profile.domain.com', 
    'scheme' => 'https', 
    'middleware'=>['Auth', 'Api']
], function(RouteBuilder $routes) {
     $routes->get/post/put.../any
})

Group supports the following option

  • host
  • method
  • scheme
  • requirements
  • defaults
  • prefix

And other common parameters that need to be stored for some custom services like:

  • middleware
  • plugin

Custom service should not be provided by routing package. route just store the paramters.

class Route
{
   protected $path;
   protected $action;
   protected $name;
   protected $method;
   //...
   protected $parameters; //Stores custom paramters for other services
}
@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 17, 2017

Member

Isn't the subcollections of routes' be covered by the existing scope methods? Adding or changing URL generation is a big enough topic that we should have a separate discussion.

Member

markstory commented May 17, 2017

Isn't the subcollections of routes' be covered by the existing scope methods? Adding or changing URL generation is a big enough topic that we should have a separate discussion.

@slince

This comment has been minimized.

Show comment
Hide comment
@slince

slince May 17, 2017

If the scope is doing the same thing, then it can of course. All method name i given are just recommend. After all my english is not good.

The generator,builder and matcher are working together. if one of them is changed, the other should be properly adjusted

@markstory

slince commented May 17, 2017

If the scope is doing the same thing, then it can of course. All method name i given are just recommend. After all my english is not good.

The generator,builder and matcher are working together. if one of them is changed, the other should be properly adjusted

@markstory

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory May 21, 2017

Member

I've spread out the agreed upon changes between the 3.5 Roadmap and 3.6 Roadmap.

Member

markstory commented May 21, 2017

I've spread out the agreed upon changes between the 3.5 Roadmap and 3.6 Roadmap.

@lukewatts

This comment has been minimized.

Show comment
Hide comment
@lukewatts

lukewatts May 22, 2017

The first two links to Issues are broken in the 3.5 Roadmap. They link to "issue/123456" instead of "issues/123456"

The first two links to Issues are broken in the 3.5 Roadmap. They link to "issue/123456" instead of "issues/123456"

@markstory markstory referenced this issue May 24, 2017

Closed

Improve ergonomics of defining routes #10689

6 of 6 tasks complete

@Kareylo Kareylo referenced this issue Oct 5, 2017

Closed

RFC : EntityRouting #11277

2 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment