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

look into proto-lens-jsonpb #109

Closed
martyall opened this issue Nov 19, 2019 · 5 comments
Closed

look into proto-lens-jsonpb #109

martyall opened this issue Nov 19, 2019 · 5 comments
Assignees

Comments

@martyall
Copy link
Collaborator

It's possible that this library could allow is to get away with less manual json definitions or WrappedVals

https://hackage.haskell.org/package/proto-lens-jsonpb

@IvantheTricourne
Copy link
Contributor

IvantheTricourne commented Nov 25, 2019

https://github.com/tclem/proto-lens-jsonpb/blob/master/src/Data/ProtoLens/JSONPB/Class.hs#L224-L226

This function might help out with types with String Fields issues as described in #92.

@IvantheTricourne
Copy link
Contributor

@IvantheTricourne IvantheTricourne self-assigned this Dec 2, 2019
@IvantheTricourne
Copy link
Contributor

I should be able to give a pretty thorough pros/cons write up for this.

See my WIP branch

@IvantheTricourne
Copy link
Contributor

IvantheTricourne commented Dec 2, 2019

Given below, I don't think we benefit using this library. Aside from not having to worry about possible null values (i.e., because every PB instance is required to declare a default field value), we actually incur writing more boilerplate and manual JSON instances.

CONS

  • we cannot use generic deriving
  • we must manually derive FromJSONPB instances via (.:) for any nullable type
  • we cannot completely remove wrapped values: certain request types (i.e., Query) actually use string parameters (i.e., height); JSON-PB only allows for string to int parsing.

PROS

  • every nullable field must have a protolens FieldDefault instance (i.e., we don't have to handle Nulls)
  • every FromJSON instance just uses (.:) and not (.?)/(.!=)
  • most ToJSON and FromJSON instances can be derived from the corresponding PB equivalent

@IvantheTricourne
Copy link
Contributor

In a similar effort of trying to use generic JSON parsers, I've come to another conclusion:

We should only use this library if we find any value in encoding defaults inside of a FieldDefault versus directly in an Aeson parser:

newtype Foo = Foo {..}

instance FromJSON Foo where
  parseJSON = withObject "Foo" $ \v -> Foo
    <$> v .:? "some_nullable_field" .!= someDefault

versus with PB:

newtype Foo = Foo {..}

instance FromJSONPB NullableField where
  parseJSONPB = ...
instance FieldDefault NullableField where
  fieldDefault = someDefault

instance FromJSON Foo where
  parseJSON = withObject "Foo" $ \v -> Foo
    <$> v .: "some_nullable_field"

@martyall martyall closed this as completed Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants