Skip to content

suggestion to replace "value" with "contents" in Either decoder, encoder #115

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

Closed
peterbecich opened this issue Nov 25, 2023 · 2 comments
Closed

Comments

@peterbecich
Copy link

peterbecich commented Nov 25, 2023

Is your change request related to a problem? Please describe.

In Haskell's Aeson library, the default contentsFieldName is "contents":
https://hackage.haskell.org/package/aeson-2.2.1.0/docs/Data-Aeson.html#v:defaultTaggedObject

In argonaut-codecs, it appears the default is "value":

val <- note (AtKey "value" MissingValue) $ FO.lookup "value" obj

Examples:
https://github.com/coot/purescript-argonaut-aeson-generic depends on argonaut-codecs and attempts to provide compatibility between PureScript's Argonaut and Haskell's Aeson. It uses "contents". For most data types, it works great. However, it cannot decode/encode Either to be compatible with Haskell's Aeson.
I have provided an example in this issue: coot/purescript-argonaut-aeson-generic#22

I have attempted to fix this on the Haskell side by overriding the instances to use "value", but haven't managed to make this work: https://github.com/peterbecich/purescript-bridge/blob/55e265b0d44c001357cd3aef3ac4a894128df12f/example/src/Types.hs#L96

Describe the solution you'd like
My request is to change "value" to "contents" here:

val <- note (AtKey "value" MissingValue) $ FO.lookup "value" obj

and in the encoder

I have tested this: eskimor/purescript-bridge@55e265b This change fixes argonaut-aeson-generic. Either is encoded and decoded successfully.

Additional context
I am attempting to update Purescript Bridge and encountered this issue: eskimor/purescript-bridge#89

Thank you

@peterbecich peterbecich changed the title suggestion to replace "value" with "contents" in Either decoder suggestion to replace "value" with "contents" in Either decoder, encoder Nov 25, 2023
@kindaro
Copy link

kindaro commented Dec 19, 2023

Hi @peterbecich and thanks for your continuing effort to merge IOHK's changes to purescript-bridge.

Please take a look at this comment nearby: #113 (comment) — and the conversation preceding it. Note the following:

I recommend being careful here. Argonaut does not promise to match Aeson, though it borrows heavily from that library. I don't expect that all instances in this library are the same as the Aeson forms, and they may diverge in the future. If you need to have a JSON library that exactly matches Aeson for use with purescript-bridge, then I think you should fork this library or create a new one with that explicit goal (that would also let you write any additional instances you want).

— Thomas Honeyman

This is bad news for us. We can fork this package under a different name, say argonaut-aeson-codecs, say that our new package promises to be compatible with Haskell's aeson, and make argonaut-aeson-generic depend on the fork. Then we shall have a solid foundation for purescript-bridge. This seems to be the most constructive way forward.

@peterbecich
Copy link
Author

Thanks @kindaro , I have read all of #113 (comment) now; I incorrectly assumed Argonaut promised to match Aeson.

I agree we should fork https://github.com/purescript-contrib/purescript-argonaut-codecs.

However, now that we have identified this issue, I believe it will be an issue for both the purescript-bridge master branch, and for my PR eskimor/purescript-bridge#89 If I'm correct about this, we should merge eskimor/purescript-bridge#89 regardless because master is currently broken anyways.

The IOHK fork implemented better test coverage. Specifically, the RoundTrip test detects the issue: https://github.com/eskimor/purescript-bridge/tree/920c23672baec3b26a92fd0ff08f52d3a7929139/test/RoundTripArgonautAesonGeneric

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

No branches or pull requests

2 participants