[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

        $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.

… format for semantic configuration (nicolas-grekas) This PR was merged into the 7.4 branch. Discussion ---------- [Config][DependencyInjection] Deprecate the fluent PHP format for semantic configuration | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This PR deprecates the fluent PHP format for semantic configuration introduced in Symfony 5.4 by `@Nyholm` (see #40600). It aims to replace it with the new array-based PHP config format (see #61490). The fluent PHP config format was a great experiment: - It helped us improve the Config component and the code generation of fluent config builders. - It confirmed the community’s interest in PHP-based configuration. - And it showed us its limits. Those limits are structural. Writing fluent config is difficult and full of edge cases. Its rigidity comes from having to match one canonical interpretation of the semantic config tree. Automatic code generation can’t capture the custom logic that before-normalizers introduce, yet those normalizers are essential for flexibility and backward compatibility. This rigidity makes fluent config fragile. How do we deal with this fragility as config tree authors? At the moment, we don't care. Maybe this format is too niche for you to have experienced this issue, but we cannot guarantee that simple upgrades won't break your fluent PHP config. The new array-based PHP format builds directly on the same code used for loading YAML configs. That means: - trivial conversion between YAML and PHP arrays, - identical flexibility and behavior, - and support for auto-completion and static analysis through generated array shapes. The generated array shapes are rigid too, but that rigidity is non-breaking: even if your config no longer matches the canonical shape, your app keeps working. Static analyzers might warn you: that’s an invitation to update, not a failure. I'm submitting this PR a bit l late for 7.4 but I think it's important to merge now. Deprecating the fluent PHP config format will: - prevent new code from relying on a fragile approach, - make room in the documentation for the array-based format, - and consolidate Symfony’s configuration story around a robust PHP-based format. Fluent PHP for semantic config served us well but it's time to retire it. ```diff -return function (AcmeConfig $config) { - $config->color('red'); -} +return new AcmeConfig([ + 'color' => 'red', +]); ``` PS: there's another fluent config format for services and routes (see #23834 and #24180). This other format is handwritten. It doesn't have the issues listed above and it is *not* deprecated. It's actually the recommended way *for bundles* to declare their config (instead of XML, see #60568). Commits ------- 332b4ac [Config][DependencyInjection] Deprecate the fluent PHP format for semantic configuration

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

nicolas-grekas mentioned this pull request Oct 17, 2025

[Config][DependencyInjection] Deprecate the fluent PHP format for semantic configuration #62092

Merged

Uh oh!

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

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

Uh oh!

Conversation

nicolas-grekas commented Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

javiereguiluz commented Sep 13, 2017

Uh oh!

nicolas-grekas commented Sep 13, 2017

Uh oh!

sstok commented Sep 14, 2017

Uh oh!

javiereguiluz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Sep 20, 2017

Uh oh!

robfrawley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajessu Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efermi commented Sep 28, 2017

Uh oh!

nicolas-grekas commented Sep 28, 2017

Uh oh!

efermi commented Sep 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nicolas-grekas commented Sep 13, 2017 •

edited

Loading

ajessu Sep 27, 2017 •

edited

Loading