Closed intolerable closed 3 years ago
Thanks. @fisx @maksbotan lets have a look at this before release.
We should at least make sure the cookbook works =)
It sounds to me like we should remove the
MimeRender ctype a => MimeRender ctype (WithStatus status a)
instance completely and keep the
ctype a => ctype (WithStatus status a)
instances for each ctype we support.
Then the instances do not overlap; but still have the correct behaviour
I think the default JSON instance for WithStatus
is very dubious too:
instance (GHC.Generic (WithStatus n a), ToJSON a) => ToJSON (WithStatus n a)
instance (GHC.Generic (WithStatus n a), FromJSON a) => FromJSON (WithStatus n a)
This should use newtype deriving instead. It requiring a Generic
constraint is very unflexible
This still doesn't work correctly in HEAD, unfortunately. There are also no tests in servant-server for WithStatus
(which I assume is why this wasn't caught).
Adding a new test for the WithStatus
combinator (and turning on -fprint-potential-instances
) still results in
/home/fraser/Code/servant/servant-server/test/Servant/ServerSpec.hs:866:16: error:
• Overlapping instances for Servant.API.ContentTypes.MimeRender
JSON (WithStatus 203 AnimalResponse)
arising from a use of ‘serve’
Matching instances:
two instances involving out-of-scope types
instance Servant.API.ContentTypes.MimeRender ctype a =>
Servant.API.ContentTypes.MimeRender ctype (WithStatus _status a)
-- Defined in ‘Servant.API.UVerb’
instance [overlappable] ToJSON a =>
Servant.API.ContentTypes.MimeRender JSON a
-- Defined in ‘Servant.API.ContentTypes’
• In the second argument of ‘($)’, namely
‘serve (Proxy :: Proxy UVerbApi) server’
In the first argument of ‘with’, namely
‘(pure $ serve (Proxy :: Proxy UVerbApi) server)’
In the expression:
with (pure $ serve (Proxy :: Proxy UVerbApi) server)
|
866 | with (pure $ serve (Proxy :: Proxy UVerbApi) server) $ do
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Correct. I just noticed this as well. Tracking here: https://github.com/haskell-servant/servant/issues/1367
Would you mind opening a PR with the failing test? I'll then push the fix on top of that
Almost any use of the new UVerb
WithStatus
combinator fails with anOverlapping instances
error, due to the (in the case of JSON)MimeRender ctype a => MimeRender ctype (WithStatus status a)
andToJSON a => MimeRender JSON a
instances. It's possible to work around this issue by writing an explicitHasStatus
instance, but this makes UVerbs usage much more verbose.Due to this issue, the UVerb cookbook page doesn't build at all.
The rest of the new UVerb work is incredible and super helpful, keep up the great work!