Named routes #1270

xsawyerx · 2016-10-10T21:28:31Z

[This related to GH #33.]

This PR allows a user to name their routes and then use a new keyword uri_for_route() to generate a path to that route with a LOT of control over how it's constructed.

Giving names:

# 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};

Using it in code:

# Assuming this is run on localhost:5000

# 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' } ); 
get NAME, PATH, sub {...};

This also works in templates:

<!-- some_template.tt -->

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

Works with route parameters
Works with typed route parameters
Works with splat
Works with megasplat
Works on GET, POST, PUT, PATCH, DELETE
Does not work with regexp routes
Does not allow multiple routes of the same name
Not supported on HEAD routes (which are created automatically with GET routes)

xsawyerx · 2016-10-10T21:31:18Z

(rebased from master)

xsawyerx · 2016-11-08T15:14:47Z

Any comments from the @PerlDancer team?

SysPete · 2016-11-08T15:19:26Z

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 · 2016-11-08T15:27:11Z

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

xsawyerx · 2016-11-08T15:28:32Z

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 · 2016-11-08T15:29:05Z

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

SysPete · 2016-11-08T15:34:18Z

Second param can already be a hashref:

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

SysPete · 2016-11-08T15:35:57Z

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

racke · 2016-11-08T16:46:49Z

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

xsawyerx · 2016-12-14T21:02:40Z

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 047d858)

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 · 2019-12-21T03:27:35Z

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

lib/Dancer2/Core/App.pm

lib/Dancer2/Core/DSL.pm

lib/Dancer2/Core/App.pm

lib/Dancer2/Core/Route.pm

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 · 2023-10-31T23:06:21Z

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 · 2023-11-01T10:19:56Z

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 · 2023-11-09T19:57:25Z

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 · 2023-11-10T15:26:37Z

Fixed all the tests.

xsawyerx · 2023-11-10T17:29:54Z

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

cxw42

Thanks very much!

Some points re. b4707f0's metadata:

the commit summary ends with a trailing colon --- is something missing from the summary line?
The author date is in 2016!
the commit message still says "This should lend in the future to allow uri_for (or a different DSL keyword)". Now that 2ff2d53 implements uri_for_route, I think this piece of the message should be removed or modified.
the commit messages says "globally incrementing number". However, it looks like 2ff2d53 changes that to leave routes anonymous unless a name is given. Therefore, I suggest removing the $count logic from b4707f0 and removing the relevant text from the commit message.

lib/Dancer2/Core/DSL.pm

lib/Dancer2/Core/Route.pm

lib/Dancer2/Manual.pod

lib/Dancer2/Core/App.pm

lib/Dancer2/Manual/Keywords.pod

lib/Dancer2/Core/App.pm

lib/Dancer2/Manual/Keywords.pod

This new DSL provides a uri_for()-style resolution, but uses named routes for this. get 'view_product' => '/view/:product/:id' => sub {...}; get 'scary' => '/*/:foo/**' => sub {...}; # somewhere else in your App my $uri = uri_for_route( 'view_product' => { 'product' => 'phone', 'id' => 'K2V3', }); # $uri = /view/phone/K2V3 $uri = uri_for_route( 'view_product', { 'foo' => 'bar', 'splat' => [ 'baz', ['quux'] ], }, { 'id' => 4 }, ); # /baz/bar/quux?id=4 * This works on any non-HEAD method (GET, POST, PATCH, PUT, DELETE, and if you create your own). * Splat and Megasplat are supported. Mixing it with named params is also supported. * Query parameters are supported. * HTML escaping is supported. * `request.uri_for_route()` in templates is also supported. * Lots of testing. * Documentation updated.

xsawyerx · 2023-11-12T18:58:20Z

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.

lib/Dancer2/Manual/Keywords.pod

cromedome · 2023-11-22T14:40:11Z

Make sure to document usage from a template:

<!-- some_template.tt -->

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

xsawyerx · 2023-11-24T11:11:17Z

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

Done.

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

cromedome · 2023-11-25T03:15:19Z

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

xsawyerx · 2023-11-25T09:25:29Z

@racke What do you think?

racke

Looks really good 👍

cromedome · 2023-12-07T15:23:03Z

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

cromedome · 2023-12-12T01:23:35Z

Ready for release. Thanks everyone!

xsawyerx added the Enhancement label Oct 10, 2016

xsawyerx force-pushed the feature/named-routes branch from fffcf0d to 6ce9112 Compare October 10, 2016 21:30

xsawyerx mentioned this pull request Oct 10, 2016

make uri_for receive a name for a route #33

Closed

cxw42 reviewed Dec 21, 2019

View reviewed changes

cromedome requested a review from SysPete January 12, 2021 03:18

cromedome modified the milestones: January 2021 Release, Future Release Jan 12, 2021

cromedome assigned xsawyerx Apr 1, 2022

cromedome added Needs Rebase This needs to be rebased (cleanly) to accept Needs Rework This needs a bit more work to be accepted labels Apr 1, 2022

xsawyerx force-pushed the feature/named-routes branch from 047d858 to cdb7d13 Compare November 9, 2023 19:47

xsawyerx force-pushed the feature/named-routes branch from cdb7d13 to 959b72f Compare November 10, 2023 15:24

xsawyerx force-pushed the feature/named-routes branch from 959b72f to 2ff2d53 Compare November 10, 2023 17:28

cxw42 approved these changes Nov 11, 2023

View reviewed changes

cxw42 reviewed Nov 11, 2023

View reviewed changes

lib/Dancer2/Core/App.pm Outdated Show resolved Hide resolved

lib/Dancer2/Manual/Keywords.pod Outdated Show resolved Hide resolved

xsawyerx force-pushed the feature/named-routes branch from 2ff2d53 to 1211dbf Compare November 12, 2023 18:09

xsawyerx force-pushed the feature/named-routes branch from 316b12d to 81c28ef Compare November 14, 2023 12:55

cromedome reviewed Nov 18, 2023

View reviewed changes

lib/Dancer2/Manual/Keywords.pod Outdated Show resolved Hide resolved

xsawyerx force-pushed the feature/named-routes branch from 81c28ef to 2a86388 Compare November 20, 2023 11:28

A set of fixes, thanks to Chris White (@cxw42)

1f702da

xsawyerx force-pushed the feature/named-routes branch from 2a86388 to 1f702da Compare November 20, 2023 11:29

Document in template

01ec74c

racke approved these changes Dec 7, 2023

View reviewed changes

cromedome merged commit 01ec74c into main Dec 12, 2023
18 checks passed

cromedome deleted the feature/named-routes branch December 12, 2023 01:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Named routes #1270

Named routes #1270

xsawyerx commented Oct 10, 2016 •

edited

Loading

xsawyerx commented Oct 10, 2016

xsawyerx commented Nov 8, 2016

SysPete commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

SysPete commented Nov 8, 2016

SysPete commented Nov 8, 2016

racke commented Nov 8, 2016

xsawyerx commented Dec 14, 2016

cxw42 commented Dec 21, 2019

xsawyerx commented Oct 31, 2023

xsawyerx commented Nov 1, 2023 •

edited

Loading

xsawyerx commented Nov 9, 2023 •

edited

Loading

xsawyerx commented Nov 10, 2023

xsawyerx commented Nov 10, 2023 •

edited

Loading

cxw42 left a comment

xsawyerx commented Nov 12, 2023

cromedome commented Nov 22, 2023

xsawyerx commented Nov 24, 2023

cromedome commented Nov 25, 2023

xsawyerx commented Nov 25, 2023

racke left a comment

cromedome commented Dec 7, 2023

cromedome commented Dec 12, 2023

Named routes #1270

Named routes #1270

Conversation

xsawyerx commented Oct 10, 2016 • edited Loading

xsawyerx commented Oct 10, 2016

xsawyerx commented Nov 8, 2016

SysPete commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

xsawyerx commented Nov 8, 2016

SysPete commented Nov 8, 2016

SysPete commented Nov 8, 2016

racke commented Nov 8, 2016

xsawyerx commented Dec 14, 2016

cxw42 commented Dec 21, 2019

xsawyerx commented Oct 31, 2023

xsawyerx commented Nov 1, 2023 • edited Loading

xsawyerx commented Nov 9, 2023 • edited Loading

xsawyerx commented Nov 10, 2023

xsawyerx commented Nov 10, 2023 • edited Loading

cxw42 left a comment

Choose a reason for hiding this comment

xsawyerx commented Nov 12, 2023

cromedome commented Nov 22, 2023

xsawyerx commented Nov 24, 2023

cromedome commented Nov 25, 2023

xsawyerx commented Nov 25, 2023

racke left a comment

Choose a reason for hiding this comment

cromedome commented Dec 7, 2023

cromedome commented Dec 12, 2023

xsawyerx commented Oct 10, 2016 •

edited

Loading

xsawyerx commented Nov 1, 2023 •

edited

Loading

xsawyerx commented Nov 9, 2023 •

edited

Loading

xsawyerx commented Nov 10, 2023 •

edited

Loading