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

PoC: omitNothingFields without INCOHERENT instances #839

Closed
wants to merge 1 commit into from

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Feb 19, 2021

The idea is dumb and simple:

(I speak only about Encoding, toJSON / Value is analogous).

  • Add a toFieldEncoding :: a -> Maybe Encoding to ToJSON class (and liftToFieldEncoding to ToJSON1 etc.)

    • This tells the Generic framework to omit the field if Nothing and omitNothingFields or convert it to Null otherwise.
  • Then behavior per type can be specified per instance, and not inside the framework:

    • Maybe
    • Strict.Maybe
    • Option
    • ()
    • ...

    Can be easily made omittable.

This is kind of adding Missing clause to Value, but without
problems that poses.

Additional bonus, which I haven't yet figured out,
is that we can have variants of object and pairs
such that nulls are omitted in hand written instances too.
Currently there is no good story for that.

The FromJSON will be analagous, we'll add

parseFieldJSON :: Maybe Value -> Parser a

Which can succeed if value is Nothing, i.e. field doesn't exist.


This is breaking change.
It fixes issue brought up many times on issue tracker, in a way I'm comfortable with.
(It removes use of IncoherentInstances).

It's however not even very breaking, as tests didn't break (so far).
So there is a chance that users of Generic (and TH) won't even notice.

Should I proceed with fleshing this out properly?

@phadej phadej requested a review from bergmark February 19, 2021 20:18
@tolysz
Copy link
Contributor

tolysz commented Feb 19, 2021

As long as it is possible to generate null or omit for the same key... it will be awesome. Adding Missing allows to generate instances... as if the leaf would be collapsed to Missing the parent object would remove it.

@phadej
Copy link
Collaborator Author

phadej commented Feb 19, 2021

As long as it is possible to generate null or omit for the same key...

I'm not sure I understand this.

@tolysz
Copy link
Contributor

tolysz commented Feb 19, 2021

On the object level we have only Maybe and we can only chose it once for every field... and there is no way to change it... we need to have 3 we have 2. Maybe (Maybe a) will never be isomorphic to Maybe a.
#646 (comment)

@phadej
Copy link
Collaborator Author

phadej commented Feb 19, 2021

@tolysz I still don't understand what you are trying to say.

@bergmark
Copy link
Collaborator

bergmark commented Feb 19, 2021

Would this allow us to have something like

data R = R
  { x :: Nullable Int -- { x: null } or { x: 3 }
  , y :: Undefineable Int -- { } or { y :: 3 }
  , z :: NullOrUndefineable Int -- { } or { z: null } or { z: 3 }
  }

?

@phadej
Copy link
Collaborator Author

phadej commented Feb 20, 2021

@bergmark

data Nullable a = N | Nullable a

instance ToJSON a => ToJSON (Nullable a) where
  toFieldJSON N            = Just Null
  toFieldJSON (Nullable a) = toFieldJSON a

data Undefinable a = U | Undefinable a

instance ToJSON a => ToJSON (Undefinable a) where
  toFieldJSON U               = Nothing
  toFieldJSON (Undefinable a) = toFieldJSON a

dat NullOrUndefined a = N' | U' | NullOrUndefined a

data NullOrUndefined a => ToJSON (NullOrUndefined a) where
  toFieldJSON U'                  = Nothing
  toFieldJSON N'                  = Just Null
  toFieldJSON (NullOrUndefined a) = toFieldJSON a

There is also variations on how wrappers process (above they doesn't) the recursive call. E.g. Undefinable could convert Nulls to "undefined"s etc. I think the design space is so large here, that these may be better left out for now. Yet, they can be defined.

@bergmark
Copy link
Collaborator

@phadej nice! go go

I agree that we can wait on defining the instances.

@tolysz does my example match your intention with "As long as it is possible to generate null or omit for the same key... it will be awesome. "?

@cdornan
Copy link

cdornan commented Jun 14, 2021

This looks like it will solve a problem of mine, and I would argue the general problem of dealing with various newtype Maybe wrappers. At the moment I am looking at cloning the Generic machinery and amending it to deal with my Maybe types.

Would it make any sense to expose the Generic machinery in an internal module to allow it to be reused by power users.

@tolysz
Copy link
Contributor

tolysz commented Jun 14, 2021

dat NullOrUndefined a = N' | U' | NullOrUndefined a Anything 3 state is fine. In my universe I used Possible. You will need to add Missing to the Object anyway, if you want to be able to deeply compose... but baby steps.

@stevladimir
Copy link

This looks very promising. What is the status of this PR?

@friedbrice
Copy link
Contributor

@cdornan, believe me! You do not want to see that code. No matter your power level, you certainly do not want to re-use it.

I've seen things you people wouldn't believe. ~Roy

This is not going to go the way you think! ~Luke

@phadej
Copy link
Collaborator Author

phadej commented Jun 12, 2023

Clsoing in favour of #1039

@phadej phadej closed this Jun 12, 2023
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

Successfully merging this pull request may close these issues.

6 participants