-
Notifications
You must be signed in to change notification settings - Fork 6
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
Record fields list #117
base: master
Are you sure you want to change the base?
Record fields list #117
Conversation
-- Even though elm supports records with no field {} we don't ever derive these | ||
-- Haskell records are in fact just type field accessors so there is ambiguity for zero fields constructors | ||
-- Usually it makes more sense to derive Single constructor type for the Elm side so that's what we do | ||
-- However it's possible to manually define instance of Elm class that would produce empty records (the other unit) type on Elm side |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important comment
|
||
encodeRecordUnit : T.RecordUnit -> Value | ||
encodeRecordUnit x = list (\_ -> null) [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decodeStrict @RecordUnit "[]"
Just RecordUnit
decodeRecordUnit : Decoder T.RecordUnit | ||
decodeRecordUnit = D.succeed {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toJSON $ RecordUnit
Array []
in theory we can fail here unless json contents empty array. Not if there will be any advantage though.
f12677f
to
1c40d6f
Compare
<> if isEmptyRecord | ||
then emptyRecordDecoder | ||
else if elmRecordIsNewtype |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really excited about this. Any idea?
data RecordUnit = RecordUnit | ||
deriving stock (Generic, Eq, Show) | ||
deriving anyclass (FromJSON, ToJSON) | ||
|
||
instance Elm RecordUnit where | ||
toElmDefinition _ = DefRecord $ ElmRecord | ||
{ elmRecordName = "RecordUnit" | ||
, elmRecordFields = [] | ||
, elmRecordIsNewtype = False | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manual instance implementation of record with no fields.
I'm not very fond of this change. |
I think it's better to represent elm types close to how elm represents it. Modeling record fields as non empty values is simply wrong. I assume one of the reasons was for support of The other advantage is that list is simpler to define than nonempty list so this also removes a bunch of noise from manual declaration of elm record (which I think is especially annoying since it should not be required). Where this might be handy later is if we get to same feature where we would start utilizing elm's extensible records for one reason or the other. I outlined one of the possible use-case in #45 (comment). I don't expect it to be something we implement but this change would be required for it as well as any other feature that would manipulate records in similar way. I don't think there is any nice elm code I can show because in the end one can always do Try to think about it. I know it might seem contra intuitive to see advantage of adding a special case but I think in long run this should lead to elimination of special cases. Especially if we get to implementing truly unboxable and type safe newtype generation. |
1c40d6f
to
a0695d3
Compare
Following the alias -> record rename #116 this PR aims to make record representation more accurate
Support for
{}
records on elm side (these are very important in elm as they're core part of extensible records). This type of record is never derived by generics instance but it can be defined by hand. This makes representation ofElmRecord
closer to real elm record which might become handy in the future.In JSON, empty record is represented by
[]
to make implementation compatible with aeson deriving.