[DX] Improve route requirements definition: allow to define requirements inline with placeholders itself #26481

Strate · 2018-03-10T12:18:47Z

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes
Symfony version	4.x

It would be great to inline requirements to pattern itself:
/route/path/{parameter:\d+} instead of /route/path/{parameter}, requirements={{parameter: "\d+"}}

The text was updated successfully, but these errors were encountered:

javiereguiluz · 2018-03-10T15:45:49Z

For reference purposes, Django framework allows something similar:

'articles/<int:year>/<int:month>/<slug:slug>/'

The special converters are int, slug, str, uuid and path, but you can use regex too:

r'^articles/(?P<year>[0-9]{4})/(?P<month>[0-9]{2})/$'

fabpot · 2018-03-10T18:14:27Z

It's been on my list of things to do forever. I would love to see somebody work on this one. Would definitely be a huge DX improvement.

javiereguiluz · 2018-03-10T18:38:46Z

We can consider the previous comment from Fabien as an approval of this idea 🎉, so now let's talk about the details before someone can try to implement it. The syntax proposed by @Strate looks nice to me:

/route/path/{param-name}
/route/path/{param-name:param-requirement-as-regex}

I'd use that simple syntax and wouldn't complicate things with something like the Django's converters.

Now, two things to think about:

What if the regex contains a :? Should we escape it ... or should we be smart and split the param by the first : and ignore the other possible occurrences of :?
What if we want to extend this idea to also define default values for params? We can't use another : as separator (/{page:\d+:1}) if we are not careful in 1).

sstok · 2018-03-11T10:55:12Z

How about using a ;? That's what I did in RollerworksAppSectioning.
https://github.com/rollerworks/app-sectioning/blob/master/docs/install.md#dynamic-host-requirements

The ; is seen as separator by most URI's but not used any reges while : could be used a regex for none-catching groups (which is almost a requirement for usage in the Route path).

linaori · 2018-03-12T07:36:44Z

Just throwing out some ideas here, because I really love this proposal:

// |
@Route("/foo/{bar|\d+|1}/")
@Route("/foo/{bar|.*}/")
@Route("/foo/{bar|[a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3}|co-uk}/")

// ;
@Route("/foo/{bar;\d;1}/")
@Route("/foo/{bar;.*}/")
@Route("/foo/{bar;[a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3};co-uk}/")

// " "
@Route("/foo/{bar \d 1}/")
@Route("/foo/{bar .*}/")
@Route("/foo/{bar [a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3} co-uk}/")

// ,
@Route("/foo/{bar,\d,1}/")
@Route("/foo/{bar,.*}/")
@Route("/foo/{bar,[a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3},co-uk}/")

// ", "
@Route("/foo/{bar, \d, 1}/")
@Route("/foo/{bar, .*}/")
@Route("/foo/{bar, [a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3}, co-uk}/")

// "; "
@Route("/foo/{bar; \d; 1}/")
@Route("/foo/{bar; .*}/")
@Route("/foo/{bar; [a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3}; co-uk}/")

// "| "
@Route("/foo/{bar| \d| 1}/")
@Route("/foo/{bar| .*}/")
@Route("/foo/{bar| [a-zA-Z0-9]{2,3}-[a-zA-Z0-9]{2,3}| co-uk}/")

nibsirahsieu · 2018-03-12T11:42:14Z

nibsirahsieu commented

Mar 12, 2018

+1 for this. Remind me a few years back when i was using play framework. Just for reference https://www.playframework.com/documentation/2.6.x/ScalaRouting#dynamic-parts-with-custom-regular-expressions

nicolas-grekas · 2018-03-13T15:52:34Z

What about this?

{bar} -- no requirement, no default value
{bar<.*>} -- with requirement, no default value
{bar?default_value} -- no requirement, with default value
{bar<.*>?default_value} -- with requirement and default value

I think the implementation shouldn't be very complex, we could put the parsing logic in Route::setPath() (and replace setDefaults/setRequirements() by addDefaults/addRequirements() in the constructor)

Anyone willing to give it a try?

fmata · 2018-03-13T16:11:33Z

I can try :)

Tobion · 2018-03-14T00:46:17Z

I like @nicolas-grekas syntax the most. But I would propose a small change

{bar=default_value}

I think this looks more what people are used to and makes it clear what the name and what the value is.

I would also recommend to do the parsing directly in Route::setPath so to the outside a route is still represented as usual with requirements and defaults. This way a route path is still compatible with URI template standard level 1 (https://tools.ietf.org/html/rfc6570#section-1.2)
I looked at the URI template standard again, but it does not define a syntax for default values or requirements (because it is only meant for generating but not matching).

Tobion · 2018-03-14T00:57:51Z

One problem I see with having the default value inside the path is that it will be hard to set a non-string default. Often times you only want to make the param optional but don't care about the default. You just need to be able to distinguish between a given value and no value given.

For example this common case

route:
    path: /blog/{slug}
    defaults:
        slug: null

Translating that to

route:
    path: /blog/{slug=null} # not clear if it means 'null' as string or null
    path: /blog/{slug=~} # has the same problem: null or '~' ?

For this common case I would propose to add a shortcut syntax

route:
    path: /blog/{?slug}

Tobion · 2018-03-14T01:06:09Z

Actually that brings be back to what nicolas proposed. With a little addition: You can leave out the default value, which means null. Sorry for dumping by brain here.

{bar} -- no requirement, no default value
{bar<.*>} -- with requirement, no default value
{bar?default_value} -- no requirement, with default value
{bar<.*>?default_value} -- with requirement and default value
{bar?} -- no requirement, with default value of null
{bar<.*>?} -- with requirement and default value of null

linaori · 2018-03-14T07:07:47Z

What will happen with something like: /foo/{bar?}/baz/ and /foo/{bar<.*>?}/baz/? I know this is currently possible, but it makes it really confusing.

nicolas-grekas · 2018-03-14T07:40:41Z

The ? just configures the default value. Being optional means both having a default value and being at a tail position. Nothing new here.

javiereguiluz · 2018-03-14T08:07:43Z

I like Nicolas proposal of using <...> for the requirement, but I'd like to ask why don't you like using a = to define the default value. It looks more natural to me:

{bar} -- no requirement, no default value
{bar<.*>} -- with requirement, no default value
{bar=default_value} -- no requirement, with default value
{bar<.*>=default_value} -- with requirement and default value
{bar=} -- no requirement, with default value of null
{bar<.*>=} -- with requirement and default value of null

And now some real-world route examples to see this in action:

# Symfony Demo app
@Route("/page/{page}", defaults={"_format"="html"}, requirements={"page": "[1-9]\d*"}, name="blog_index_paginated")
@Route("/page/{page<[1-9]\d*>}", defaults={"_format"="html"}, name="blog_index_paginated")

# Symfony Demo app
@Route("/admin/post/{id}", requirements={"id": "\d+"}, name="admin_post_show")
@Route("/admin/post/{id<\d+>}", name="admin_post_show")

# Twig bundle app
@Route("/{code}.{_format}", requirements={"code": "\d+"}, defaults={"_format"="html"}, name="_twig_error_test")
@Route("/{code<\d+>}.{_format=html}", name="_twig_error_test")

linaori · 2018-03-14T08:11:09Z

I agree that in a real world example, it looks a lot better with =. However, the >= is slightly confusing, but imo not a deal breaker.

mvrhov · 2018-03-14T08:31:54Z

Maybe both of these cases should be made valid. So You don't have to remember which way it is....

{bar<.*>=default_value} -- with requirement and default value
{bar=default_value<.*>} -- with requirement and default value

{bar<.*>=} -- with requirement and default value of null
{bar=<.*>} -- with requirement and default value of null

linaori · 2018-03-14T09:01:12Z

{bar=default_value<.*>} slightly reminds me of generics notation wise, kinda "feels right"

nicolas-grekas · 2018-03-14T09:08:17Z

Here we are: #26518

{bar=default_value<.*>} -- with requirement and default value

The default value is a string, why should it be parsed? Not a good idea to me as it creates more ambiguities than it should.

why don't you like using a = to define the default value

because ? better conveys the idea that there is something conditional there, while = conveys the idea that there is some affectation, which might happen, but only conditionally (thus ? is a more important idea to convey IMHO)

javiereguiluz · 2018-03-14T09:12:45Z

because ? better conveys the idea that there is something conditional there, while = conveys the idea that there is some affectation

OK. I just wanted to avoid issues if some day Symfony adopts the URI Template RFC, where ? is used a lot.

…defaults (nicolas-grekas) This PR was merged into the 4.1-dev branch. Discussion ---------- [Routing] Allow inline definition of requirements and defaults | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26481 | License | MIT | Doc PR | - ``` {bar} -- no requirement, no default value {bar<.*>} -- with requirement, no default value {bar?default_value} -- no requirement, with default value {bar<.*>?default_value} -- with requirement and default value {bar?} -- no requirement, with default value of null {bar<.*>?} -- with requirement and default value of null ``` Details: * Requirements and default values are not escaped in any way. This is valid -> `@Route("/foo/{bar<>>?<>}")` (requirements = `>` and default value = `<>`) * Because of the lack of escaping, you can't use a closing brace (`}`) inside the default value (wrong -> `@Route("/foo/{bar<\d+>?aa}bb}")`) but you can use it inside requirements (correct -> `@Route("/foo/{bar<\d{3}>}")`). * PHP constants are not supported (use the traditional `defaults` syntax for that) * ... Commits ------- 67559e1 [Routing] Allow inline definition of requirements and defaults

javiereguiluz added Feature Routing DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Mar 10, 2018

nicolas-grekas mentioned this issue Mar 14, 2018

[Routing] Allow inline definition of requirements and defaults #26518

Merged

nicolas-grekas closed this as completed Mar 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DX] Improve route requirements definition: allow to define requirements inline with placeholders itself #26481

[DX] Improve route requirements definition: allow to define requirements inline with placeholders itself #26481

[DX] Improve route requirements definition: allow to define requirements inline with placeholders itself #26481

[DX] Improve route requirements definition: allow to define requirements inline with placeholders itself #26481

Comments