AutoRoute doesn't work with non-primitive types well

[neongreen]

I am using IHP 0.14.

I have the following controller:

data ReplySource = ReplySourceCard { cardId :: Id Card } | ReplySourceInbox
    deriving (Eq, Show, Data)

data ReplyController
    = NewReplyAction { cardUpdateId :: Id CardUpdate, replySource :: ReplySource }
    | CreateReplyAction { cardUpdateId :: Id CardUpdate, replySource :: ReplySource }
    | EditReplyAction { replySource :: ReplySource, replyId :: !(Id Reply) }
    | UpdateReplyAction { replySource :: ReplySource, replyId :: !(Id Reply) }
    | DeleteReplyAction { replySource :: ReplySource, replyId :: !(Id Reply) }
    deriving (Eq, Show, Data)

I also have a view using NewReplyAction: <a href={NewReplyAction (get #id cardUpdate) replySource} ...>.

The generated action looks like this:

http://localhost:8000/NewReply?cardUpdateId=35e54da1-a897-4cda-8193-6856ac214bcb&replySource=ReplySourceCard%7BcardId%3D03462af6-e31d-44fd-a48b-ac253552a8c4

The closing } is missing from ReplySourceCard{cardId=... in the URL. However, even if I add the closing }, I get the following:

Query parameter replySource needs to be a UUID but got ReplySourceCard{cardId=03462af6-e31d-44fd-a48b-ac253552a8c4}
Routing failed with: BadType {expectedType = "UUID", value = Just "ReplySourceCard{cardId=03462af6-e31d-44fd-a48b-ac253552a8c4}", field = "replySource"}

Looks like #763, but #763 is supposed to be fixed.

[neongreen]

AutoRoute works with the following parameter types:

Text, [Text], Maybe Text, Int, [Int], Maybe Int, Id (for all model types)

Oh, alright.

Would be nice if this was typechecked, though.

[neongreen]

Would also be nice if AutoRoute was using smth like class AutoRouteParam instead of Data (with an overlapping instance going via Data), because currently I don't think I have any way to override the parsing for ReplySource without reimplementing AutoRoute for the whole controller by hand.

[zacwood9]

Hi there, I wrote the typed auto route system so I'm happy to address any concerns with this. This is a very tricky problem to solve in Haskell currently because to build a feature like AutoRoute, we need to take an arbitrary URL and choose a data constructor from a given type. Using this constructor, we then need to take arbitrary ByteStrings and attempt to fill in the fields needed for that constructor.

As far as I'm aware, there are a few ways to go about this currently in Haskell: GHC.Generics, Data.Data (current solution), or some Template Haskell solution. Each of these has trade offs, but we decided the Data.Data solution was best. Using Data.Data, we can take a type and get representations of all its constructors as runtime. If the route matches one of the constructors' names, we take that constructor and fold over its fields using gfoldl from Data.Data. We pass this function a function who's param (type d) is constrained to a Data instance. This is as much info as we can get for each field of the constructor -- some arbitrary type d that we know has a Data instance.

Now the problem arises of taking a query parameter and constructing a value of type d. To do this, I took the list of types above and check using Data.Typable if d happens to be any of those types. If it is we try and construct that type from the query param string.

Ideally we would like any type with a Read instance to be used in AutoRoute. However, to the best of my knowledge, this is not possible with Data.Data, since in order to fold over a constructor we are constrained to types of Data only and have no way of getting "every type with a read instance".

I am by no means a Haskell expert so please let me know if you think any of this is incorrect, and if it would be possible to do something like you're suggesting! It would be really awesome if we could use custom data types :)

[neongreen]

GHC.Generics is a bit tricky to use, but something like generics-eot should be pretty simple. I wrote a tutorial here: https://tek.brick.do/generics-are-simple-write-your-own-to-json-9AM0Bz8yBZBP.

class Param a where parseParam :: ...
instance Param Text where ...
instance Param Int where ...

class AutoRoute controller where
  autoRoute :: Parser controller
  default autoRoute :: (HasEot controller, GAutoRoute (Eot controller)) => Parser controller

Eot will simply represent the action as an Either branch with a tuple inside. You will also have access to the constructor name.

After that, GAutoRoute will use Param for the individual fields. The upside is that anybody can write their own Param instance or use deriving via to specify that they want to go via Read/Show:

deriving Param X via (ParamViaShow X)

Where ParamViaShow will be a newtype wrapper provided by IHP.

[neongreen]

The downside of using runtime dispatch instead of compile-time dispatch is exactly that overriding things becomes hard. You can supply a new Param instance trivially, but you can't trivially supply an extra branch case into a function deep in IHP.

[neongreen]

In fact, you can't do that even non-trivially, because custom Typeable instances aren't supported. (Otherwise I would've just written a custom Data instance that lies to AutoRoute.)

[zacwood9]

That looks pretty nice! What do you think @mpscholten?

[neongreen]

Btw I'll be happy to code this up in a stream on Saturday — although first I'd like to hear why you decided against GHC.Generics. Maybe what I'm proposing is harder than I think.

[neongreen]

(generics-eot reuses GHC.Generics)

[zacwood9]

I believe Marc had mentioned to me earlier versions of IHP experimented with GHC.Generics but got hit by really bad compile times. I'm sure he can tell you more - my additions were adding onto and extending the existing Data.Data solution.

That would be awesome if you could code it! When I started working on the typed auto route it was much trickier than I thought it would be - a lot of the balance comes in keeping a very nice and simple user facing API while providing the advanced type features. Not always easy to balance but I'm confident we can do something much much better than the current solution

[mpscholten]

Yes, we mostly banned and removed all usage of Generics in the IHP code base because of compile time issues. Generics affect compile times in a quadratic way. In the beginning when you have only very few data types it all looks good, once the application is growing it will seriously slow down everything. Specifically live reloading only works when compiling doesn't take ages. So generics also break the live reloading experience when your application is growing big enough.

[neongreen]

Alright.

Another option is to create a global registry that will be used during runtime dispatch — imagine a top-level IORef with a typerep-map inside. Handlers for specific types could be added in a centralized place somewhere in the application, and the dispatch inside AutoRoute would throw a descriptive error if the type isn't found in the registry.

[neongreen]

This would also likely let you get rid of autoRouteWithIdType — instead, the codegen will have to register a single Id' x handler per model.

[neongreen]

I've looked at the sources of AutoRoute some more.

First of all, the arguments parser that tries to parse Show-generated output is buggy — it doesn't work when one of the params contained inside the controller is a record. This can be fixed by using one of the available Show-output parsers, e.g. shower or pretty-show or pretty-simple. I recommend doing this regardless, just because a) it's going to remove a piece of annoying code from the IHP codebase and is going to be b) strictly less buggy.

Secondly, using Show on non-primitive types is (mostly?) useless if you don't use Read to read those non-primitive types back. So you'd need a way to get a Read instance at runtime when parsing the query back into the controller.

You can't use Read controller => as an AutoRoute constraint because it won't give you Read constraints for the controller fields. You could do something like All Read (GetFields controller), but this requires generics.

I think that a working option is to use https://github.com/litxio/instance-map, which is implemented via TH and will require just one TH invocation (per codebase, not per type) to allow you to query things like "give me the Read instance if it exists" inside the parser.

If you add Read/Show instances for Id', which you can do because you control all generated Id' types, you won't need autoRouteWithIdType either.

If you want to give users control over rendering non-primitive parameter fields, you shouldn't use Read and Show at all and replace them with a QueryParam class that would provide both a parser and a renderer. This will be a breaking change, but since you're not using Read, all user-written code that actually uses the Show fallback is also probably failing at runtime.

[neongreen]

I am also curious if you had evaluated generics-sop when deciding to ban generics. generics-sop has a TH-based generator that doesn't rely on GHC.Generics, and the resulting generics instances are much faster to compile (while being correspondingly slower at runtime, but Data.Data has the same story).

[mpscholten]

generics-sop might be a good solution, we've only used the normal generics 👍

[neongreen]

We switched from GHC.Generics to generics-sop at Juspay and there was a significant improvement in compilation time — but it's hard to say whether the slowdown after switching from Data.Data will be negligible, or 1.5x, or 2x, or who knows what. At least they are not quadratic-time, though.

[CSchank]

If this is going to be something that would be possible, I'm all for it, since I have myself been confused trying to use a non-supported type with AutoRoute.

If this will take a long time to change, or isn't worth supporting for whatever reason (be it the quadratic problem or another reason like code complexity), can we try to make this into a type error or some sort of compiler error? Maybe that list of supported types could be encoded in a typeclass which only has instances for the supported types. I know it was confusing for me to have this break at runtime, and I'm an experience Haskeller. New people could be pretty thrown off by this I think.

[neongreen]

I think we can't make this a compile-time error without either generics (normal/sop/etc) or TH.

[zacwood9]

https://www.parsonsmatt.org/2021/09/09/deferred_derivation.html Read this blog post today and it reminded me of the issues we are facing here. This seems really promising - I believe with the techniques mentioned here, we'd be able to just add one line of Template Haskell in each FrontController file that would "discover" all the AutoRoute instances imported and generate some truly type safe routing for them. Since they have very few dependents it shouldn't affect compile times too much either.

I don't have the capacity for this currently, but figured I'd put this here in case anyone else wanted to try ^_^