Proposal for Phoenix: add HTTP verb to VerifiedRoutes

Hello everyone!

tldr; I propose to add a way for VerifiedRoutes to know if they’re used for get, post, etc to make them even more robust, and safer to refactor. This solution could be to add a “tag” to verified routes, like so: ~p"/user/#{user}"get.

Context

So, I’ve been using Phoenix for a while, both personally and professionally, and was reaaaaally glad when verified routes were introduced. Compared to the router helpers, they are shorter and easier to read, safer, and help ease your mind when introducing new routes.
There is however one shortcoming I have come across multiple times already. Let’s create some context:
I’ve got two routes with different verbs, but with the same name

# router.ex

get "/users/:id", UserController, :show
put "/users/:id", UserController, :update

Let’s say I want to rename the put route to end with /edit to match a new :edit route

get "/users/:id", UserController, :show
get "/users/:id/edit", UserController, :edit
put "/users/:id/edit", UserController, :update

This change is never going to be caught by verified routes, whilst I think it could be really handy, at the very least for large applications (and for control freaks, like the one I am).

Proposal

Although I do have a proposal, my main intention is to solve this issue I’ve been having quite some times already, so feel free to propose other options :blush:

The simplest way I found to fix this issue would be to add a “tag” to verified routes. It uses an existing feature of sigils, and from what I saw shouldn’t be too hard to implement.

~p"/users/#{user}"get # Still works after refactor
~p"/users/#{user}"put # Emits a warning after refactor, since `put "/user/:id"` does not exist anymore

It also has the advantage of being easily made optional

~p"/users/#{user}" # matches anything, making existing verified routes exactly the same as before

The most obvious drawbacks I can see for this proposal are

  • the process is manual, making it pretty error-prone (it’s easy to copy-paste a route and forget to update the tag)
  • I personally think it decreases readability of verified routes, but I think that it’s still an acceptable trade-off.

As a side note, I though about having an option to enforce usage, but I think that this should be the role of a linter like Credo, more than the library itself.

Sooo, here’s for the proposal. It’s my very first one so, sorry if I missed any obvious information :smile: I’m willing to try to implement this if this is something that you think is interesting.
Thanks for your time, and have a nice day :wave:

5 Likes

One drawback to this is that the url might not be directly correlated to an actual request being made. Yes you validate that a put request endpoint exists, but the code might do a patch instead - even just for an otherwise totally valid refactor. I would still consider this a useful addition especially given it‘s opt in.

2 Likes

I opened PR #5805 to make a proposal of implementation :blush:

I didn’t answer as I don’t have much to add to the topic, but I completely agree with this limitation @LostKobrakai (actually, that’s what I meant by “the process is manual …”, but reading it again, I get the sentence was incorrect and didn’t convey what I meant :grimacing:). Still, I just can’t think of a better way that would remove the need for manual validation.

This is a good idea. However, I’d recommend this syntax:

~p"PUT /users/#{user}"
4 Likes

I agree this is better—combining single letter flags that have no meaning on their own feels sloppy to me. It’s too bad we don’t have multi-letter lowercase sigils so it could be ~put"/users/#{user}". Not sure if that is planned or not (I never read the discussion around the uppercase ones).

@zachdaniel I like the idea, thanks for the input ! I saw ~p must always start with a / right now, so it should be fairly easy to differentiate both usages.

@sodapopcan There’s one big limitation with this syntax (~put"..."). HTTP verb can be anything, Phoenix supports matching custom ones - you can do match :custom, "/posts/custom", PostController, [] and the route will only match if the verb is CUSTOM. We can’t generate sigils for that (I mean, we could likely find a solution, but I don’t think it’s worth looking into it)

2 Likes

I updated the pull request to use the syntax proposed by @zachdaniel. I kept the commit history to keep the first syntax just in case, but I’ll remove the relevant commits if this new syntax is chosen to stay :smile:

1 Like