Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToSchema, proxy, and GeneralizedNewtypeDeriving #150

Closed
michalrus opened this issue Mar 22, 2018 · 7 comments
Closed

ToSchema, proxy, and GeneralizedNewtypeDeriving #150

michalrus opened this issue Mar 22, 2018 · 7 comments

Comments

@michalrus
Copy link
Contributor

michalrus commented Mar 22, 2018

This won’t work:

{-# LANGUAGE GeneralizedNewtypeDeriving #-}

newtype SomeId =
  SomeId UUID
  deriving (Show, Eq, Ord, ToJSON, FromJSON, ToJSONKey, FromJSONKey, ToSchema, ToField, FromField)

with the following error:

    • Couldn't match representation of type ‘proxy UUID’
                               with that of ‘proxy SomeId’
        arising from the coercion of the method ‘Data.Swagger.Internal.Schema.declareNamedSchema’
          from type ‘forall (proxy :: * -> *).
                     proxy UUID
                     -> Data.Swagger.Declare.Declare
                          (Data.Swagger.Internal.Definitions Data.Swagger.Internal.Schema)
                          Data.Swagger.Internal.NamedSchema’
            to type ‘forall (proxy :: * -> *).
                     proxy SomeId
                     -> Data.Swagger.Declare.Declare
                          (Data.Swagger.Internal.Definitions Data.Swagger.Internal.Schema)
                          Data.Swagger.Internal.NamedSchema’
      NB: We cannot know what roles the parameters to ‘proxy’ have;
        we must assume that the role is nominal
    • When deriving the instance for (ToSchema SomeId)
   |
44 |   deriving (Show, Eq, Ord, ToJSON, FromJSON, ToJSONKey, FromJSONKey, ToSchema, ToField, FromField)
   |                                                                      ^^^^^^^^

Perhaps when using a concrete Data.Proxy.Proxy instead of forall proxy. proxy, this would’ve worked?

@phadej
Copy link
Collaborator

phadej commented Mar 22, 2018

Alternatively we could have TaggedNamedSchema a. which might be even better..

@phadej
Copy link
Collaborator

phadej commented Mar 22, 2018

@michalrus note that GND is questionable for ToSchema though, as the name of the schema won't change, it will be still UUID. It might be what you want, or not.

@michalrus
Copy link
Contributor Author

Ohhhhhh, right, absolutely! I don’t want that. Sorry for the noise!

@fieldstrength
Copy link
Contributor

Hi! I recently found myself affected by this design decision as well. I would be quite interested if there is some way to have a workaround. If so I'd be willing to help with the implementation.

My use case involves DerivingVia. I have a newtype that keeps track of how to transform the JSON encodings. This is quite valuable to us because it prevents the various type classes that know about the encoding from getting out of sync.

data MyRecord = MyRecord 
  { ...
  } deriving stock Generic
    deriving (FromJSON, ToJSON, Flow, ToSchema) via APIEncoding SnakeCase MyRecord

This is what I want to write (see here for a gist). Everything works beautifully until we get to ToSchema for the reason mentioned in this issue: the type parameter role is assumed to be nominal.

Now about the solution:

@phadej It sounds like you had an idea, but it is not immediately clear to me what it is. Maybe you could illuminate? Something involved the Tagged type?

I suspect there would be compatibility concerns around simply changing the function signature to take a concrete Proxy (is anyone using the fact that its polymorphic?) so I suppose we could add a new method with this signature, and require that either one be specified. Admittedly it does seem a bit unfortunate to pollute the class in this way though.

Is there a better way?

@phadej
Copy link
Collaborator

phadej commented Feb 22, 2019 via email

@fizruk
Copy link
Member

fizruk commented Feb 22, 2019

@fieldstrength I know this is not as neat as DerivingVia, but you can always do this:

instance ToSchema Example where
  declareNamedSchema _ = declareNamedSchema (Proxy @ (APIEncoding SnakeCaseDropPrefix Example))

See the full gist (based on yours).

@phadej how much code would we break if we switch from proxy to Proxy? I don't really see the benefit of proxy anymore, do you?

@fieldstrength
Copy link
Contributor

Thanks for the feedback and pointers!

I was encouraged @fizruk that you seem positive towards the change so I went ahead and made the PR. Happily, it is a quite small patch. I have also sanity-checked that it does indeed allow DerivingVia to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants