Closed amesgen closed 1 year ago
@amesgen It's a fantastic addition, thank you very much! Tell me if you need help with the doctest CI :)
Tell me if you need help with the doctest CI :)
Yeah, I think I could use some help there; I can reproduce the CI failure locally, but I am a bit mystified as I don't modify Servant.API.TypeLevel
and it does not use named routes :thinking: I will try to tinker with it a bit more, but feel free to e.g. directly push to this branch if you see what is going wrong.
Maybe it is due to something like https://github.com/sol/doctest/issues/327? Personally, I've been using cabal-docspec ever since I became aware of it as it was much more robust for me.
Can't blame you, cabal-docspec really is the best one out there.
Moved the custom error message to the TypeErrors
module; which seems to somehow have fixed the doctest failures.
@amesgen Thanks for your contribution ! I find it very helpful.
As this approach relies non-trivially on type family evaluation semantics, I can't outrule that this might yield type errors in contexts where one tries to use
NamedRoutes
with a not-yet-concrete api, but playing around, I couldn't find anything, and a relatively large internal code base usingservant
and various add-on packages also compiled fine.
So I am guessing this will only be a problem if one tries to call serveWithContext
on NamedRoutes routes
where routes
itself isn't concrete. I don't think that's a common use-case, and adding constraints on Rep (routes ())
into context might be enough to unstuck the type family application and solve the issue. So I think it is safe to merge this PR.
If you forget to derive
Generic
for your API types in the context of named routes, you get rather unhelpful error messages. For example, considerwhich yields
for me on GHC 9.2.4. One might spot the
Rep
type family here fromGHC.Generics
, but it is certainly not obvious at all, the first message in particular. This can be particularly bad when you have many nested APIS, residing in completely different modules.In this PR, we use a trick from this blog post: https://blog.csongor.co.uk/report-stuck-families/
With it, the first error message gets much simpler:
The error messages for other snippets such as
also improve in the same manner.
I added the error-reporting constraint as a superclass constraint to the various instances of
NamedRoutes
. There might be further places to apply this pattern, or even a way to also make the second error message better (naively addingErrorIfNoGeneric
to e.g.GenericServant
does not work, as then certain call sites oftoServant
/fromServant
then no longer compile).As this approach relies non-trivially on type family evaluation semantics, I can't outrule that this might yield type errors in contexts where one tries to use
NamedRoutes
with a not-yet-concreteapi
, but playing around, I couldn't find anything, and a relatively large internal code base using servant and various add-on packages also compiled fine.