urlFor() is a misnomer; proposal for pathFor() and/or an actual urlFor() in 3.x

[rszrama]

I'm taking the time to learn Slim 3.x after learning about it from Josh, and I came across this function in the Router. It's very helpful, but it's a misnomer in that it doesn't actually provide URLs for named routes. It provides paths to those routes with argument replacement. It lacks the protocol or domain name that would be required for the return value to be considered a URL.

To remedy this, I propose at least renaming the existing function to pathFor(), which is still very useful i its own right. If you wanted, you could then also provide a urlFor() function that includes the protocol and domain in its return value. (I wouldn't recommend doing what Drupal has done by overloading url() to return either a path by default or a full URL if 'absolute' => TRUE.)

I'm happy to submit a pull request in this direction but figured it best to get feedback before doing so. :smile:

[codeguy]

I have no objections. @rszrama Also got your email. Will get back to you tomorrow when I have some time to write a decent response :)

[rszrama]

:+1:

[geggleto]

I am okay with renaming it to pathFor.

[Melbournite]

In general, I am okay with it. It probably makes sense. But should it wait for a later release?

I know it's not yet entered beta or any RC stage and this is when we're meant to change stuff, but the fact is people have found it stable enough to start using, myself included. Not only this, there's already a few tutorials, videos etc. floating around with the urlFor() function used. Renaming it now would essentially break them... Easy to fix I realize, but at the end of the day we're only changing a word.

On the other hand, the inconvenience is only for early users, rather than potentially many more later down the path.

[rszrama]

Carpe diem! https://github.com/slimphp/Slim/pull/1320

I hear ya on the "it will bother the people using the framework now", so it seems the best thing to do here will be to just rename the function and not add an actual urlFor(). This allows anyone currently affected to do a simple search / replace and everyone in the future to tighten up their language.

(fwiw, I've maintained a few projects with a lot of early adopters whose livelihoods I've held in my git repos, so I'm not ignoring the inconvenience; it just seems like now is still the appropriate time for a change like this to be made, and the developers affected will be the most competent to address it.)

[Melbournite]

If changed, Twig-View is going to have to be updated too, which relies on urlFor. Not sure if other service providers do also.

[rszrama]

I just saw that as well. I'm new here, so I didnt know to check the other slimphp repos to assess impact. Are they being kept up to date with develop here?

[akrabat]

This would be a BC break for the Slim 2 upgraders for no real benefit from what I can tell.

[silentworks]

I don't think a name change should be made to this function at all, it has been this name since the dawn of Slim and a change like this with no real benefit but semantics seem a bit of an overkill.

Some history on this function, if I remember correctly the name originated from the Flask python framework's url_for and some functionality were not ported across to keep things simple. But having look over the Flask's version again, I think we would be better off adding an extra parameter to compensate for the absolute url being provided. In Flask this function is also used for loading static assets at which point it can just be a path that get's provided.

You can have a look at the Flask version here https://github.com/mitsuhiko/flask/blob/master/flask/helpers.py#L186 and you can see the extra param here https://github.com/mitsuhiko/flask/blob/master/flask/helpers.py#L247.

My suggestion would be to add an extra param for the user to get an absolute url.

[rszrama]

New major versions are the right time for backwards compatibility breaks.

I would argue that semantic precision is important. It communicates careful thinking to new adopters of a technology. In this case, I believe what urlFor() returns is not a URL. If that's the case, why maintain the dissonance simply because prior versions introduced it?

I mentioned above the Drupal approach of a parameter overloading the url() function to accommodate multiple return types. I believe this is bad API design that would be better addressed by separate functions.

There are plenty of discussions about "absolute" vs "relative" URLs on the web, however, and I'm willing to admit the case may be made based on the W3C URL draft spec (https://url.spec.whatwg.org) that urlFor() could be described as returning a subset of their conception of a relative URL (which they actually refer to as a "path", fwiw, as opposed to a "scheme-relative URL" that includes the domain). The IETF doesn't seem to make a similar accommodation (http://tools.ietf.org/html/rfc3986#section-1.1.3), so I believe these bodies are at odds, with the W3C's recommendation being driven by the historic usage of URLs in a tags and their stipulation that in order to be used, a base URL must be in scope.

Anyways, I think there's solid support in the standards here, just like using named route mappers conforms to the standards for HTTP method definitions. I won't lose any sleep over this, but I will say that I at least as a newcomer was thrown off by the name vs. the return value.

(And re: BC breaking, I believe there are other changes to the definition of routes / patterns that will require much more accommodation than a search / replace from one name to the next. : )

[codeguy]

I agree new versions are the time to do this. If everyone is against this, we might introduce a new uriFor() that aliases the existing urlFor() method. We mark the urlFor() as scheduled for removal in next version, and we encourage everyone to use the uriFor() method.

[akrabat]

I can see @rszrama argument & I agree about the extra parameter. it's harder to make mistakes with two methods compared to a single one with a flag.

In Slim 3, urlFor is a bit of a weird function at the moment as it is aware of query parameters, but not of the scheme name, domain name or base path. I suppose that's why Slim2 has a urlFor in the application class which is at least aware of the base path.

[akrabat]

Having thought some more, Like @codeguy, I'm okay with this too.

I think it'd be nice to have an easy way to build a fully qualified URL from a route name at some point.

[Melbournite]

For me personally, if this is to happen I think deprecating the function with the alias is a nice way to go as suggested by @codeguy.

[akrabat]

Would the alias trigger an E_USER_DEPRECATED?

[silentworks]

I think just state deprecated in docbloc and allow it to work as it currently does, should probably not trigger anything IMO. We should also note it will be removed in a minor release rather than the next major, although I kinda think this breaks semver.

[codeguy]

Slim is 100% semver, so it cannot be removed until 4.0. We should probably trigger a E_USER_DEPRECATED, too, like @akrabat suggested.

[akrabat]

merged #1320

[rszrama]

Sweet, my first little commit here. : )

Also, never knew about trigger_error(). Ya learn something new every day...