anuragsoni / routes

typed bidirectional router for OCaml/ReasonML web applications
https://anuragsoni.github.io/routes/
BSD 3-Clause "New" or "Revised" License
145 stars 11 forks source link

Add map and path prefixing to targets #121

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi again!

We're making heavy use of this library, and these additions would help us, if you think they make sense (happy to think again about the choice of "/~" as an operator name).

anuragsoni commented 3 years ago

Hi @Chattered ,

Thank you for making the proposal. The technical changes itself seem fine, but i'm curious about the usecase they intend to solve. If possible would you be able to give some examples of how your project intends to make use of these functions? I only ask because while /~ might be convenient in some cases, i'm not sure the burden of adding another infix operator might be worth it if its possible to achieve a similar behavior with the current set of operators. I'm thinking of something like:

let make_route p1 p2 = p1 / p2;;
let r1 = int / str;;
let r2 = s "foo" / s "bar";;
let r2 = make_route r2 r1 /? nil

On a similar note, the change to map seems technically correct, but I'm curious about how you intend to use it?

One way i reuse routes with the current api is by using thunks when defining the routes:

let r () = s "sum" / int / int /? nil;;

let sum_as_int = r () @--> fun a b -> a + b;;

let sum_as_string = r () @--> fun a b ->
Printf.sprintf "Sum of %d and %d = %d" a b (a + b);;

The thunk looks awkward indeed, and I think adding an operator to make working with that easier would be nice.

I'm mostly saying all this just to start a discussion and I hope you don't take this as me rejecting this proposal 😄

ghost commented 3 years ago

Hey @anuragsoni.

In our setup, we've got 400+ endpoints, each partially defined by a record which has a field for the target. We later attach handlers, and after that, we do some namespacing of the endpoints by prefixing various paths. I suppose we could replace the target field with one of type (('a,'b) path -> ('a,'b) path) -> ('c,'d) target which takes the prefix and produces the target. My concern there is that we're already getting bogged down by type variables, and I'm wary that such functions can do a lot more than just prefixing, and I'd rather have the assurance that prefixing is all that's happening. I think we could also use a 'a router router with the outer router using a wildcard to pass on the suffix to the inner router, though that's going to involve splitting and joining the suffix for each match?

The mapping is more of an issue, just because of the way we've decomposed this application. We've got targets like (int -> handler, 'a) target and (int -> string -> handler, 'a) target with concrete handlers, and then in a routing module, we want to transform these handlers before building our actual router. I think we can get rid of the decomposition and just inline the mapping, but I'd be a bit sad about that.

My personal philosophy is that if I can see how a type constructor is a functor (in the mathematical sense), then it's worth including the map function, and so allow for the sort of decomposition we're doing. Admittedly, I might be coming from a Haskell bias here, where a Functor instance doesn't have any real impact on the size of an interface.

Thanks for the quick feedback!

anuragsoni commented 3 years ago

Thanks for the insight @Chattered

My concern there is that we're already getting bogged down by type variables, and I'm wary that such functions can do a lot more than just prefixing, and I'd rather have the assurance that prefixing is all that's happening.

I think this is enough of a justification that supporting this in the library is justified :) Thanks for providing the examples of how you use it!

The mapping is more of an issue, just because of the way we've decomposed this application. We've got targets like (int -> handler, 'a) target and (int -> string -> handler, 'a) target with concrete handlers, and then in a routing module, we want to transform these handlers before building our actual router. I think we can get rid of the decomposition and just inline the mapping, but I'd be a bit sad about that.

This makes sense. I don't see any problem with supporting a map operation in the library.

happy to think again about the choice of "/~" as an operator name

I'm going to merge this PR in its current state, but I'll probably spend some time in thinking about the operators to see if we can come up with something different that might be easier to read.

In our setup, we've got 400+ endpoints, each partially defined by a record which has a field for the target. We later attach handlers, and after that, we do some namespacing of the endpoints by prefixing various paths.

Thanks for sharing this. I really appreciate getting to learn about your usage pattern as this sounds bigger than most applications i've heard of that use routes.

ghost commented 3 years ago

Awesome! Thanks again.