[Routing] Add PHP fluent DSL for configuring routes #24180

nicolas-grekas · 2017-09-13T06:45:52Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

If we add a PHP DSL for DI config (#23834), we must have a similar one for routing. Here it is. See fixtures.

So, you always start with a RoutingConfigurator $routes, which allows you to:

$routes->add($name, $path); // adds a route
$routes->import($resource, $type = null, $ignoreErrors = false); // imports routes
$routes->collection($name = ''); // starts a collection, all *names* might be prefixed

then

for "import" and "collection", you can ->prefix($path);
for "add" and "collection", you can fluently "add" several times;
for "collection" you add sub"->collection()";
and on all, you can configure the route(s) with:

->defaults(array $defaults)
->requirements(array $requirements)
->options(array $options)
->condition($condition)
->host($pattern)
->schemes(array $schemes)
->methods(array $methods)
->controller($controller)

javiereguiluz · 2017-09-13T08:05:41Z

I like this proposal! Thanks for contributing it.

My only super tiny concern is about a variable name. I propose to rename RoutingConfigurator $route to RoutingConfigurator $routes I know it's just a single letter difference, but to me it changes its meaning completely:

// add something to the given route
$route->add(...);

// add a route to the given collection of routes
$routes->add(...);

A more real comparison using your fixture:

// BEFORE
return function (RoutingConfigurator $route) {
    $route
        ->add('foo', '/foo')
            ->condition('abc')
            ->options(array('utf8' => true))
        ->add('buz', 'zub')
            ->controller('foo:act');

    $route->import('php_dsl_sub.php')
        ->prefix('/sub')
        ->requirements(array('id' => '\d+'));

    $route->add('ouf', '/ouf')
        ->schemes(array('https'))
        ->methods(array('GET'))
        ->defaults(array('id' => 0));
};

// AFTER
return function (RoutingConfigurator $routes) {
    $routes
        ->add('foo', '/foo')
            ->condition('abc')
            ->options(array('utf8' => true))
        ->add('buz', 'zub')
            ->controller('foo:act');

    $routes->import('php_dsl_sub.php')
        ->prefix('/sub')
        ->requirements(array('id' => '\d+'));

    $routes->add('ouf', '/ouf')
        ->schemes(array('https'))
        ->methods(array('GET'))
        ->defaults(array('id' => 0));
};

nicolas-grekas · 2017-09-13T11:51:52Z

@javiereguiluz fixtures updated.

sstok · 2017-09-14T13:52:01Z

The collection will make it so much easier to add routes with a common prefix without having to store those routes in a separate file! I love this 👏

return function (RoutingConfigurator $routes) {
    $news = $routes->collection('news_')
        ->prefix('news/')
        ->requirements(['id' => '\d+'])
    ;

    // No need to repeat the name prefix, the requirements are applied here but
    // are still part of the same file.
    $news('show', '{id}')->controller('News:show');
    $news('edit', '{id}/edit')->controller('News:edit');
    $news('delete', '{id}/delete')->controller('News:delete');
};

javiereguiluz

I like it! Thanks for this contribution.

javiereguiluz · 2017-09-20T06:30:38Z

src/Symfony/Component/Routing/Loader/Configurator/CollectionConfigurator.php

+    }
+
+    /**
+     * Sets the prefix to add the path of all child routes.


This description looks like it's missing something. What about?

Sets the prefix added to the path of all child routes.

javiereguiluz · 2017-09-20T06:31:39Z

src/Symfony/Component/Routing/Loader/Configurator/ImportConfigurator.php

+    }
+
+    /**
+     * Sets the prefix to add the path of all child routes.


Same comment here.

javiereguiluz · 2017-09-20T06:34:45Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

+     *
+     * @return $this
+     */
+    final public function schemes(array $schemes)


Would you consider using variadics here and in methods() method too?

javiereguiluz · 2017-09-20T06:35:20Z

src/Symfony/Component/Routing/Loader/PhpFileLoader.php

@@ -37,7 +38,21 @@ public function load($file, $type = null)
        $path = $this->locator->locate($file);
        $this->setCurrentDir(dirname($path));

-        $collection = self::includeFile($path, $this);
+        // the closure forbids access the private scope in the included file


forbids access the private scope -> forbids accessing the private scope or forbids access to the private scope ?

nicolas-grekas · 2017-09-20T06:51:50Z

@javiereguiluz thanks, comments addressed.

Would you consider using variadics here and in methods() method too?

I wouldn't, for the same "least surprise" reason. If someone wants to push stronger for variadics, I suggest doing so in a dedicated PR, after this one (and the DI one) have been merged.

robfrawley

This looks great! 👍

stof · 2017-09-20T11:15:17Z

src/Symfony/Component/Routing/Loader/Configurator/CollectionConfigurator.php

+class CollectionConfigurator
+{
+    use Traits\AddTrait;
+    use Traits\RouteTrait;


shouldn't ->defaults, ->requirements and so on apply on $this->collection in this class ?

That's the case, but not on the collection yes, because it works reverse: calling on the collection means changing routes declared before. Here, we want first to configure the route prototype ($this->route in this class), then use this prototype as a template for new routes added.
The goal is to make this work as expected:

$collection->defaults(['foo' => 'bar'])->add(...);

but I'm not sure doing $collection->add(...); then $collection->defaults(['foo' => 'bar']); and having it applied to previously declared routes in the collection is something we want to have.

…icolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [Routing] Add PHP fluent DSL for configuring routes | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - If we add a PHP DSL for DI config (#23834), we must have a similar one for routing. Here it is. See fixtures. So, you always start with a `RoutingConfigurator $routes`, which allows you to: ```php $routes->add($name, $path); // adds a route $routes->import($resource, $type = null, $ignoreErrors = false); // imports routes $routes->collection($name = ''); // starts a collection, all *names* might be prefixed ``` then - for "import" and "collection", you can `->prefix($path)`; - for "add" and "collection", you can fluently "add" several times; - for "collection" you add sub"`->collection()`"; - and on all, you can configure the route(s) with: ```php ->defaults(array $defaults) ->requirements(array $requirements) ->options(array $options) ->condition($condition) ->host($pattern) ->schemes(array $schemes) ->methods(array $methods) ->controller($controller) ``` Commits ------- f433c9a [Routing] Add PHP fluent DSL for configuring routes

ajessu · 2017-09-27T13:43:19Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

+     *
+     * @return $this
+     */
+    final public function controller($controller)


I had an idea looking at the examples on the main blog post about this feature (which looks great btw): http://symfony.com/blog/new-in-symfony-php-based-configuration-for-services-and-routes

Is it possible, or does it make sense to change the api of this controller method (or even adding a new helper method), to enable use Controller::class, without having to concatenate the results?

If I'm not mistaken, all other class methods accept a FQN class name as its parameter, so you can pass a string, or use the ::class (which I find less error prone since it's IDE auto completed).

This one however, since it's following the config that already exist would end in being something like:

->add('homepage', '/') ->controller('App\Controller\DefaultController::index')

or

->add('homepage', '/') ->controller(DefaultController::class . '::index')

In the later scenario, would changing this method's parameters or adding a new method that would accept the method of the controller separately help, as far as DX goes?

as an example:

->add('homepage', '/') ->controller(DefaultController::class, 'index')

I understand that wouldn't match the full string config for a controller, in relation to other configurations, so that might be a con, and concatenating honestly doesn't look that bad either, just wanted to raise the case, since it's different from all others, in case it makes sense.

@ajessu That's indeed a very good idea 👍 of course this action parameter should be optional as more advanced developers use a single action per controller (called Action controllers).

that's already possible: just use the array-callable PHP syntax:
->controller([DefaultController::class, 'index'])

Thanks @nicolas-grekas. That seems perfect. Wasn't aware that was a possibility here.

efermi · 2017-09-28T19:02:03Z

    $routes
        ->add('foo', '/foo')
            ->condition('abc')
            ->options(array('utf8' => true))
        ->add('buz', 'zub')
            ->controller('foo:act');

This is too fluent, code formatters will break the indent-based semantics and the interface looks confusing at the first glance.

nicolas-grekas · 2017-09-28T21:43:17Z

If you don't want to be too fluent, you can just do:

    $routes->add('foo', '/foo')
            ->condition('abc')
            ->options(array('utf8' => true));
    $routes->add('buz', 'zub')
            ->controller('foo:act');

It's up to you.

efermi · 2017-09-28T21:54:11Z

Of course, I understand that, just wanted to share my first impressions; it is a welcome change after all.

nicolas-grekas added this to the 3.4 milestone Sep 13, 2017

carsonbot added Status: Needs Review Routing Feature labels Sep 13, 2017

nicolas-grekas force-pushed the routing-dsl branch 2 times, most recently from 0b4198c to 836348f Compare September 13, 2017 07:13

nicolas-grekas force-pushed the routing-dsl branch from 836348f to a28e2c1 Compare September 13, 2017 11:51

nicolas-grekas mentioned this pull request Sep 20, 2017

[DI] Add "PHP fluent format" for configuring the container #23834

Merged

javiereguiluz approved these changes Sep 20, 2017

View reviewed changes

[Routing] Add PHP fluent DSL for configuring routes

f433c9a

nicolas-grekas force-pushed the routing-dsl branch from a28e2c1 to f433c9a Compare September 20, 2017 06:49

chalasr approved these changes Sep 20, 2017

View reviewed changes

ogizanagi self-requested a review September 20, 2017 07:30

robfrawley approved these changes Sep 20, 2017

View reviewed changes

stof reviewed Sep 20, 2017

View reviewed changes

stof approved these changes Sep 20, 2017

View reviewed changes

nicolas-grekas merged commit f433c9a into symfony:3.4 Sep 20, 2017

nicolas-grekas deleted the routing-dsl branch September 20, 2017 13:06

ajessu reviewed Sep 27, 2017

View reviewed changes

javiereguiluz mentioned this pull request Oct 3, 2017

Added new fluent PHP config formats for services and routes symfony/symfony-docs#8462

Closed

This was referenced Oct 9, 2017

[3.4] Completion for PHP-based configuration for services Haehnchen/idea-php-symfony2-plugin#1041

Open

[3.4] Completion for PHP-based configuration for routes Haehnchen/idea-php-symfony2-plugin#1042

Open

Haehnchen mentioned this pull request Oct 9, 2017

[3.4] Index for PHP-based configuration for routes Haehnchen/idea-php-symfony2-plugin#1044

Open

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

Tobion mentioned this pull request Jun 28, 2019

[Routing] deprecate RouteCollectionBuilder #32240

Closed

TomasVotruba mentioned this pull request Jul 26, 2020

[ConfigTransformer] add routing support deprecated-packages/migrify#121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Routing] Add PHP fluent DSL for configuring routes #24180

[Routing] Add PHP fluent DSL for configuring routes #24180

nicolas-grekas commented Sep 13, 2017 •

edited

Loading

javiereguiluz commented Sep 13, 2017

nicolas-grekas commented Sep 13, 2017

sstok commented Sep 14, 2017

javiereguiluz left a comment

javiereguiluz Sep 20, 2017

javiereguiluz Sep 20, 2017

javiereguiluz Sep 20, 2017

javiereguiluz Sep 20, 2017

nicolas-grekas commented Sep 20, 2017

robfrawley left a comment

stof Sep 20, 2017

nicolas-grekas Sep 20, 2017

ajessu Sep 27, 2017 •

edited

Loading

sstok Sep 27, 2017

nicolas-grekas Sep 27, 2017

ajessu Sep 27, 2017

efermi commented Sep 28, 2017

nicolas-grekas commented Sep 28, 2017

efermi commented Sep 28, 2017

[Routing] Add PHP fluent DSL for configuring routes #24180

[Routing] Add PHP fluent DSL for configuring routes #24180

Conversation

nicolas-grekas commented Sep 13, 2017 • edited Loading

javiereguiluz commented Sep 13, 2017

nicolas-grekas commented Sep 13, 2017

sstok commented Sep 14, 2017

javiereguiluz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Sep 20, 2017

robfrawley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajessu Sep 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efermi commented Sep 28, 2017

nicolas-grekas commented Sep 28, 2017

efermi commented Sep 28, 2017

nicolas-grekas commented Sep 13, 2017 •

edited

Loading

ajessu Sep 27, 2017 •

edited

Loading