Named routes

[xsawyerx]

[This related to GH #33.]

You can now give names to the routes:

get NAME, PATH, sub {...};

Or the lesser-known one:

get NAME, PATH, OPTIONS_HASHREF, sub {...};

The PATH, as before, can be a string (matching our spec) or a full regular expression.

If you do not give a name, a globally incrementing number will be used. I'm not sure that's the best idea but it's a unique identified that, if leaks, does not say which App this is part of.

This should lend in the future to allow uri_for (or a different DSL keyword) to use the name of a route instead of the path:

End result should look something like this:

# MyApp.pm
get 'view_product', '/view/product/:id' => sub {...};

# in template:
<% uri_for( 'view_product', { id => 4 } ); %>

We're getting there...

[xsawyerx]

(rebased from master)

[xsawyerx]

Any comments from the @PerlDancer team?

[SysPete]

So does this mean if the user does:

uri_for( '1', { id => 4 } );

and expects something like http://example.com/1?id=4 they are not going to get what they expect?

[xsawyerx]

Wouldn't you have to provide uri_for( '/1' ) if you wanted to get http://example.com/1?

[xsawyerx]

We can also try to either revise the syntax, like:

uri_for( { '1' => { 'id' => 4 } } );

Or add a new keyword:

named_uri_for( '1', { 'id' => 4 } );

[xsawyerx]

We can also detect that if the second parameter is a hashref, the first parameter is a name, not a path.

[SysPete]

Second param can already be a hashref:

sub uri_for {
    my ( $self, $part, $params, $dont_escape ) = @_;
    ...
    $uri->query_form($params) if $params;

[SysPete]

I think I'd prefer a new named_uri_for keyword just to be sure weird stuff doesn't happen by accident.

[racke]

Yeah a new keyword would be less confusing and prevents nasty surprises.

[xsawyerx]

Alright, so update:

In introduced a new keyword for the uri_for. It's called uri_for_route.

However: (and following is a summary of 047d858129f9d4dfa60b91fa2da5214e2794b213)

• You cannot send it the query parameters as you could for uri_for.
• The only parameters currently used are for route parameters. (So, splat/megaplat isn't supported.)
• The "don't escape" option is not supported.
• I haven't exposed this to the template yet, because I'm not sure how. Suggestions welcomed.

[cxw42]

This looks very handy! Any news? Has anyone been running on this branch and noticed any gotchas?

[xsawyerx]

Alright, rethinking this a bit, I find the most confusing part the conflation between URI parameters and Route parameters:

uri_for() is meant to create URIs and doesn't know or care about routes, so it can only support query parameters:

uri_for_route() is meant to understand routes, but it means it needs to support both route parameters and query parameters.

uri_for( '/foo', { 'id' => 4 } ) # /foo?id=4

get 'foo_with_id' => '/foo/:id' => sub {...};
uri_for_route( 'foo_with_id', { 'id' => 4 } ) # /foo/4

Notice query parameters were not supported here in uri_for_route. What if you want to create /foo/4?bar=baz?

uri_for_route( 'foo_with_id' => { 'id' => 4 }, { 'bar' => 'baz' } ) # /foo/4?bar=baz

That feels like quite a lot. I'm not really happy with it, but I'm not sure how to shorten here and keep it clean.

Then there's handling splat and megasplat, which provide their route parameters without names:

get 'foo' => '/foo/*' => sub {...};
uri_for_route( 'foo', [ 'baz' ] ) # /foo/baz

I'm proposing this as the syntax.

Them mixing them:

uri_for_route( 'foo', [ 'baz' ], { 'id' => 4 } ) # /foo/baz?id=4

At the very least, the query parameters are unlikely to be used in conjunction with named parameters because named parameters usually replace query parameters. I still feel like they need to be supported instead of ignored.

Last item to address is the method. I chose GET as the only supported one, but since we're replacing route parameters, there's no reason to not support other methods:

post 'update_entry' => '/entry/edit/:id' => sub {...};
uri_for_route( 'update_entry' => { 'id' => 4 } ) # /update_entry/4

However, now the query parameters are arguably wrong to be used:

post 'update_entry' => '/entry/edit/:id' => sub {...};

# You shouldn't use POST /entry/edit/4?foo=bar
uri_for_route( 'update_entry' => { 'id' => 4 }, { 'foo' => 'bar' } )

However, I don't want to start being restrictive and check what method it is to determine if we throw or get mad about the query parameters being provided.

What we can do is support the third argument being missing:

# Both using the "don't escape" flag option
uri_for_route( 'update_entry' => { 'id' => 4 }, 1 );
uri_for_route( 'get_entry' => { 'id' => 4 }, { 'color' => 'blue' }, 1 );

Lastly, we need to make sure there can only be one route with the same name, as pointed out by @cxw42 in a comment.

post 'update_entry' => '/entry/edit/:id' => sub {...};
patch 'update_entry' => ... # die "route with this name already existing"

I think this covers all my thoughts.

[xsawyerx]

Addendum: Routes defined with regexp directly cannot have their arguments expanded. This would require uri_for_route() to crash if it was defined as a regexp.

get 'entry' => qr{/view/[0-9]+} => sub {...};

uri_for_route( 'entry', ... ); # Boom!

And in case this seems like splat-like positional expansion would be the answer, consider regexps in routes are absolutely arbitrary and can be even superpositions:

get 'wild_entry' => qr{ / (view | display)? / ( [0-9]+ | [0-9+]/?<status> )? }x => ...;

I would've made it throw when trying to define a regexp route with a name, but I can understand the value of giving them all names, even for consistency and readability, not to mention introspection.

[xsawyerx]

Okay, rebased and rewrote everything.

Here is what we have now:

# Assuming this is run on localhost:5000

get 'test1' => '/:foo' => sub {1};
get 'test2' => '/*/**' => sub {1};
get 'test3' => '/:foo/:bar/*' => sub {1};
get 'test4' => '/:foo[Str]' => sub {1};

# ...

# Route parameters
# $path = http://localhost:5000/bar
$path = uri_for_route( 'test1', { 'foo' => 'bar' } ); 

# Route parameters with query parameters
# $path = http://localhost:5000/bar?id=4
$path = uri_for_route( 'test1', { 'foo' => 'bar' }, { 'id' => 4 } ); 

# Splat and Megasplat args
# $path = http://localhost:5000/foo/bar/baz
$path = uri_for_route( 'test2', [ 'foo', [ 'bar', 'baz' ] ] );

# Mixed arguments
# $path = http://localhost:5000/1/2/hello
$path = uri_for_route( 'test3', { 'foo' => 1, 'bar' => 2, 'splat' => ['hello'] } ); 

# Escaping content
# $path = http://localhost:5000/bar?id=he%2Flo
$path = uri_for_route( 'test1', { 'foo' => 'bar' }, { 'id' => 'he/lo' } );

# Types route parameters
# $path = http://localhost:5000/bar
$path = uri_for_route( 'test1', { 'foo' => 'bar' } ); 

Lots of tests, updated documentation.

I'm also testing for various failures:

• You tried to create multiple routes with the same name
• You failed to include parameter values for all the route parameters
• You tried calling uri_for_route() to a nonexistent route
• You provided an incorrect splat/megasplat arguments compared to how many were necessary

Last things to check before fully signing off:

• CI tests failures
• Support in template (because that's when it would be really useful)

[xsawyerx]

Fixed all the tests.

[xsawyerx]

Using it in templates is now supported as well:

<!-- some_template.tt -->

[% request.uri_for_route( 'my_route_name', { 'foo' => 'bar' }, { 'id' => 4 } ) %]

(This is an example with all possible arguments.)

[xsawyerx]

I appreciate your thorough review!

Thanks very much!

Some points re. b4707f0's metadata:

* the commit summary ends with a trailing colon --- is something missing from the summary line?

This is a common practice of mine. When a commit summary line has a colon, it means I have commit text. When I don't add a commit content, I don't end it with a colon.

So, on purpose.

* The author date is in 2016!

Yup. I amended, which kept this data.

* the commit message still says "This should lend in the future to allow `uri_for` (or a different DSL keyword)".  Now that [2ff2d53](https://github.com/PerlDancer/Dancer2/commit/2ff2d530efb677c1e237532e42ecb22a3c10d0a0) implements `uri_for_route`, I think this piece of the message should be removed or modified.

This is, to me, a piece of history. I intended to put it in uri_for() but couldn't. I think it's fine.

* the commit messages says "globally incrementing number".  However, it looks like [2ff2d53](https://github.com/PerlDancer/Dancer2/commit/2ff2d530efb677c1e237532e42ecb22a3c10d0a0) changes that to leave routes anonymous unless a name is given.  Therefore, I suggest removing the `$count` logic from [b4707f0](https://github.com/PerlDancer/Dancer2/commit/b4707f05f7a023c68037be142c0437415aa3cb94) and removing the relevant text from the commit message.

Same.

[cromedome]

Make sure to document usage from a template:

<!-- some_template.tt -->

[% request.uri_for_route( 'my_route_name', { 'foo' => 'bar' }, { 'id' => 4 } ) %]

[xsawyerx]

<!-- some_template.tt --> [% request.uri_for_route( 'my_route_name', { 'foo' => 'bar' }, { 'id' => 4 } ) %]

Done.

Also updated the article. I think it's ready to merge.

[cromedome]

👏 This looks great. Thanks for walking me through this. An emphatic 👍 from me.

[xsawyerx]

@racke What do you think?

[cromedome]

Excellent! I'll work on getting a release put together over the next couple of days. Thanks everyone!

[cromedome]

Ready for release. Thanks everyone!