make uri_for receive a name for a route

[tmueller]

This is something for the wishlist, not an issue itself.

Right now uri_for converts a relative path to an absolute path and attaches parameters if necessary.

In case you have to change a path in your controller, you have to search all your templates, to find where this path was used and change your templates accordingly.

My idea would be to provide a name for a route and use that name to identify the route to generate a uri. Something along the lines:

# in the controller
get '/product/:id' as 'product' => sub { ... }

# OR
get '/product/:id' called 'product' => sub { ... }


# meanwhile in the template ...
<a href="[% uri_for('product', { id => 5 }) %]">...</a>
<a href="[% uri_for('product', { id => 6, special_price => 1 }) %]">...</a> 

# ... becomes
<a href="/product/5">...</a>
<a href="/product/6?special_price=1">...</a>

The Benefit would be, that we can change the route in our controller without having to search our templates for where this route was used. Like so:

# in the controller
get '/product/:id/:special_price?' as 'product' => sub { ... }


# no change in the template is required, but now ...
<a href="[% uri_for('product', { id => 6, special_price => 1 }) %]">...</a>  

# ... would yield
<a href="/product/6/1">...</a>

The idea is, to decouple the uris in templates from routing in the controller, so that you are more flexible to change your routes. And to make uri generation more comfortable.

[DavsX]

For fun I'm tackling this issue right now. I was thinking about the possibilities to implement it.

I was thinking about modifying some D2::C::DSL methods, namedly GET, POST, PUT, PATCH, HEAD, OPTIONs etc to at the end return the $regex path. After that we could create a keyword that wraps those HTTP verbs, like:

named 'my_route' => get '/path' => sub {}

This way the keyword named would receive the params 'my_route' and '/path' (because the get keyword returns the path at the end) and we could get that into a hash, no problem. The problem is with the uri_for method. It would make sense NOT to create a separate named_uri_for, but use a common uri_for for both named and unnamed matches. The problem is, that the D2::C::Request package, where the uri_for lies has no awareness about it's surrounding, it does not have a reference to app nor dsl, so I have trouble getting the hash of named matches there. Any thoughs on this?

BTW I know, that the HTTP verb keywords (get, post etc) take an $options hash as an optional parameter, where technically could the route name go, but that hash is reserved for HTTP header options, so I'd rather not mix it in there.

Maybe a prettier syntax could be achieved, but that would require some internal changed in Dancer2.

[xsawyerx]

Funny that I'm looking at it too right now and didn't remember there was an issue for it already.

I was thinking of calling it a route alias. I like the get '/' as 'main' => sub {... }; but it might be trickier to handle, since get would have to receive a sub instead of a sub and definition. as could create a sub but it won't add the metadata to the route.

An easier way to handle it before the get, but I'm not sure what would be the best syntax for it. @mickeyn and I originally thought of alias 'product' => get '/'' => sub {...} but perhaps that's not the best.

I'd be really happy to get more suggestions here.

[DavsX]

route "home" => get '/' => sub {} or uri "home" => get '/' => sub {}
I'd read it like: the 'value' of the 'home' route/uri is this get route definition with the given sub...I tackled the issue long time ago, but as far as I remember putting this logic before get would make it easier to implement and would not require change in the existing route DSL. I'm just not sure about the syntax. Maybe be more verbose and make it 'route_alias'?

Now that I think about it, if this would not fit into the Core, it could be easily a plugin as well..

[xsawyerx]

route and uri are too generic. This definitely needs some thinking over.

I think this might not easily be done in a plugin because you would have to have access to aliases in the dispatching mechanism. It'd be easier to put it in core by adding another attribute to the route and then fetching that from the dispatcher.

[xsawyerx]

I've been playing with this tonight, and given #1088 merging, the following gist provides a simple implementation of it.

I would be happy to put this in a plugin, or possibly in core, but we would need to resolve two things:

• The new keyword for defining these (went with named_route for the example).
• Whether to use uri_for or provide a new uri_for method. I think the main one can be used if we're adding this to core.

There are two additional cases I'm not sure how and if to handle:

1. get creates a GET and HEAD. The latter is ignored. Correct, I'm assuming. Am I missing something?
2. What should this behave with any? Currently it will warn and return empty. Maybe should croak?

[yanick]

First and foremost, I like the idea. Makes things a little more tidy
then the uris accumulate. :-)

Thoughts and suggestions:

• If you want to incoporate this into core,
  you don't even need a registery of the routes like
  in the gist, but can add a 'named_route' attribute
  to the route itself.

• You don't really need to filter out the HEAD when it's called
  on 'get', as both the HEAD and GET will have the same route specs.
  If you want to make sure no-one do something like

  named_route "foo",
  get( '/something' => { } ),
  get( '/else' => { } );

you can do around like 29 of the gist:

use List::MoreUtils qw/ uniq /;

croak "multiple routes defined for $name"
    if @routes > uniq map { $_->spec_route } @routes

• the regex of line 20 can be buggy if params are overlapping (say
  you have the parameters foo and fool). You might want to change it
  to

  $route_path =~ s{:$param(/|$)}{$vars->{$param}$1}g;

• Supporting the splats and megasplats would be easy. I can provide code for it.
  The way to call it could be

  named_route_uri( foo => { splat => [ @args ]  );
  
  # shortcut for the one above
  named_route_uri( foo => [ @args ]  );
  

• For the names of the keywords, I like named_route $name, ...; for
  the declaration. And named_route_uri for the function that gives back the
  path?

• named_route_uri should also be able to take care of the path params and
  the dont_escape flag of uri_for. So its signature should be

  named_route_uri( $name => $route_params, %path_params, $dont_escape );

where $route_params is an optional hashref or an arrayref, and %path_params
and dont_escape are optional.

[yanick]

btw, the second bullet above also takes care of any, which will work just fine (as all its routes will share the same route specs)

[xsawyerx]

If we were considering this for core, adding it as an attribute in the Route object would be easier. I was trying to make it decoupled from the Route object. Giving an identifier for Routes I think makes sense, and I've suggested that in #1088.

Two additional quick comments:

• Good catch on the regex. I would use the same regexp used in Core::Route: /:([^\/\.\?]+)/.

• Splat and megasplat shouldn't be supported with a key in the same hash. Even though we deprecated the usage of captures and splat in the route spec, I think it creates a confusing interface. I was thinking of another hashref. Not sure, really.

  Eventually I figured that splats and megasplats shouldn't be supported, simply because they contradict the idea of the named uri_for. It's meant for named parts, not unnamed ones. By having names, you can move these around but keep the intent with names.. You can't do the same with unnamed ones.

[yanick]

On 2016-01-02 12:14 PM, Sawyer X wrote:

Splat and megasplat shouldn't be supported with a key in the same hash.
Even though we deprecated
https://github.com/PerlDancer/Dancer2/blob/master/lib/Dancer2/Core/Route.pm#L216
the usage of |captures| and |splat| in the route spec, I think it
creates a confusing interface. I was thinking of another hashref. Not
sure, really.

Then use * as the key in the hash. I think it's fair to say that :*
is not a valid named parameter.

Eventually I figured that splats and megasplats shouldn't be supported,
simply because they contradict the idea of the named |uri_for|. It's
meant for named parts, not unnamed ones. By having names, you can move
these around but keep the intent with names.. You can't do the same with
unnamed ones.

:-( That gives me the sads, as it prevents things like

named_route 'creature',
get '/classification/**' => sub {
    ...;
}

# later on in template

my $uri = named_route_uri( creature => [qw/
    Animalia
    Chordata
    Mammalia
    Carnivora
    Caniformia
    Ursidae
    Ailuropoda /, 'A. melanoleuca'
]);

[xsawyerx]

On Sat, Jan 2, 2016 at 6:49 PM, Yanick Champoux

On 2016-01-02 12:14 PM, Sawyer X wrote:

Splat and megasplat shouldn't be supported with a key in the same hash.
Even though we deprecated
https://github.com/PerlDancer/Dancer2/blob/master/lib/Dancer2/Core/Route.pm#L216
the usage of |captures| and |splat| in the route spec, I think it
creates a confusing interface. I was thinking of another hashref. Not
sure, really.

Then use * as the key in the hash. I think it's fair to say that :*
is not a valid named parameter.

That's a good idea! That can solve that part.

Eventually I figured that splats and megasplats shouldn't be supported,
simply because they contradict the idea of the named |uri_for|. It's
meant for named parts, not unnamed ones. By having names, you can move
these around but keep the intent with names.. You can't do the same with
unnamed ones.

:-( That gives me the sads, as it prevents things like

It wasn't a final decision, just what I figured at the time. :)

Happy to be find myself wrong.

named_route 'creature',
get '/classification/**' => sub {
...;
}

later on in template

my $uri = named_route_uri( creature => [qw/
Animalia
Chordata
Mammalia
Carnivora
Caniformia
Ursidae
Ailuropoda /, 'A. melanoleuca'
]);

Hmm... is this a practical example?

Aren't you just using ** and named_route_uri for simply doing a
uri_for("/Animalia/Chodata/Mammalia/Carnivora/Caniformia/Ursidae/Aliuropoda")?

[yanick]

On 2016-01-02 03:13 PM, Sawyer X wrote:

my $uri = named_route_uri( creature => [qw/
Animalia
Chordata
Mammalia
Carnivora
Caniformia
Ursidae
Ailuropoda /, 'A. melanoleuca'
]);

Hmm... is this a practical example?

It's not from a real app, but it's something I do from time to time.
There are two types of megasplats that I would use in RESTish routes.
When I have an object that has a strict but not always identical
hierarchical structure. Like the animal classification above, or say

/location/MilkyWay/Sol/Earth/Canada/Ontario

And there is the case where I can pass many values, which number and
order don't matter. E.g.

route '/entry/:id/tags/**'

for

/entry/alpha/tags/foo/bar/baz

Aren't you just using |**| and |named_route_uri| for simply doing a
|uri_for("/Animalia/Chodata/Mammalia/Carnivora/Caniformia/Ursidae/Aliuropoda")|?

Nope. Just like the other named routes, I could decide to rebase
'creature' from /creature/** to /life_form/**.

It's not a as-common case as the named parameters, but actively
preventing it will just restrict its use without any meaningful gain.

[xsawyerx]

Alright. Convinced. :)

We still need to pick the best naming for everything. Coding is easy, naming is hard. :/

[tmueller]

There are only two hard things in Computer Science: cache invalidation and naming things.
Phil Karlton

I like named_route for defining a routes name a lot. It is short and concise.
To generate URIs the most expressive name I come up with is uri_for_named_route. But it is also very long.
named_route_uri is shorter and yet expressive.

[xsawyerx]

named_uri_for ?

[tmueller]

Sounds good. Maybe named_uri and named_uri_for makes it clear, that both keywords are to be used in conjunction. But named_route and named_uri_for would be ok, too.

[tmueller]

Hi Sawyer X,

I know, you are busy being the new perl Pumpkin. But are there any knews on this issue?

[xsawyerx]

We're pushing hard to finish the plugins work for this release (hopefully going out on Sunday, tomorrow).

I can try and prepare a sample to test out today and tomorrow. I can't promise. Otherwise, it will have to wait until the next release.

Once this next release it out, it will be easier to continue making more releases. This last one took us a long long time to get exactly right, which is why no production releases were done for a while.

[xsawyerx]

@mickeyn suggested adding a set of keywords to be named, such as named_get, named_post, named_del, etc. How does that sound to people?

# in code
named_get display_product => '/view/product/:id' => sub { ... };

# in template
uri_for( display_product => { id => 33 } );

(If uri_for has one parameter, it's a path. If more, it's a named path.)

[tmueller]

I was just asking. Really, don't rush it. Do this releases plugins work and if this feature makes it into the next release, it will be just fine.

I like named_get, named_post, named_out, named_del too. As long as the intent is clear enough, I can live with it. And I think named_... is clear enough.

I don't know about uri_for though. Doesn't already understand more than one parameter? I thought, the second parameter already does what I initially described? (turns a hash into parameters)

[xsawyerx]

Let's bump this back up! :)

[xsawyerx]

I've given this a bit more thought. I think we don't need new DSL keywords.

There are only two ways to call get:

get '/' => sub {...}
get '/' => {...}, sub {...}

We can also add the following:

get display_product => '/' => sub {...}
get display_product => '/' => {...}, sub {...}

We can easily find out how many were sent. The last elements should be CODE or CODE and HASHREF. That's it. Since this is in registration time, you don't pay for it in run-time.

[tmueller]

Simple an elegant, I like it. Should also work with any product => ['get', 'post'] ...

[xsawyerx]

I wrote the sample code. I'll put up a PR for discussion.

[xsawyerx]

PR pushed: #1270. :)

[xsawyerx]

I updated the PR with a new keyword: uri_for_route, but it's still not perfect.

[xsawyerx]

This is now officially resolved as implemented and released in version 1.1.0, available on CPAN. :)

[tmueller]

Thanks @xsawyerx for implementing and releasing.