purescript-contrib / purescript-argonaut-generic Goto Github PK

Environment

• PureScript [version] 0.13.8
• purescript-argonaut-generic [version] 0.6.0

Current behavior

data NodeContent
  = NodeContentStart
  | NodeContentEnd { returnValue :: String }

encoding :: Encoding
encoding = defaultEncoding { tagKey = "tag", unwrapSingleArguments = true, valuesKey = "values" }

instance encodeJsonNodeContent :: EncodeJson NodeContent where
  encodeJson = genericEncodeJsonWith encoding

instance decodeJsonNodeContent :: DecodeJson NodeContent where
  decodeJson = genericDecodeJsonWith encoding

If there is an argument unwrapSingleArguments makes values and object, instead of an array objects.
If there is no argument, values will be an empty array.

Expected behavior

I'd expect an empty object if there are no arguments (for consistency).

toJsonString NodeContentStart 
-- Actual: "{\"values\":[],\"tag\":\"NodeContentStart\"}"
-- Expected: "{\"values\":{}\,\"tag\":\"NodeContentStart\"}"

Code

I hope the example clarifies:

data Foo = Foo Bar
data Bar = Bar

derive instance genericFoo :: Generic Foo
derive instance genericBar :: Generic Bar

instance encodeJsonFoo :: EncodeJson Foo where
  encodeJson = genericEncodeJson Argonaut.options

instance encodeJsonBar :: EncodeJson Bar where
  encodeJson _ = fromString "yippie"

main :: forall e. Eff (console :: CONSOLE | e) Unit
main = Console.log $ Json.stringify $ encodeJson $ Foo Bar

-- I expected "yippie" to show up in the output

I've also created a repro repo: https://github.com/felixSchl/repro-encodejson

Is your change request related to a problem? Please describe.
As described in the documentation section of the Library Guidelines, Contributors libraries are expected to have mostly complete module documentation, preferably uploaded to Pursuit.

This library currently doesn't have sufficient module documentation -- many publicly-exported types and functions don't have any documentation comments.

Describe the solution you'd like
The library needs to have documentation comments on the majority (and preferably all) public types and functions. Once updated, we should tag a release and upload new module documentation to Pursuit so that folks can use it.

Additional context
See the Governance repository for more information about requirements in the Contributors organization.

Is your change request related to a problem? Please describe.
The README suggests that this package can be installed with spago install argonaut but argonaut-generic is not a dependency of argonaut so this isn’t actually possible.

Describe the solution you'd like
We should either remove the instruction or add argonaut-generic to the dependencies of argonaut.

We renamed the Data.Generic.Rep.* modules to Data.*.Generic when porting generics-rep to prelude in purescript/purescript-prelude#235, so we should probably rename Data.Argonaut.Decode.Generic.Rep to Data.Argonaut.Decode.Generic, Data.Argonaut.Encode.Generic.Rep to Data.Argonaut.Encode.Generic and Data.Argonaut.Types.Generic.Rep to Data.Argonaut.Types.Generic when dropping generics-rep and updating the prelude dependency to its latest version.

Is your change request related to a problem? Please describe.
As described in the documentation section of the Library Guidelines, Contributors libraries are expected to have some documentation in the docs directory -- specifically, a changelog and at least a short tutorial that expands on the quick start in the README.

This library currently doesn't have an up to date changelog or any extra documentation in the docs directory.

Describe the solution you'd like
The changelog needs to be updated for at least the most recent release (we can then start curating it going forward). In addition, we need a short tutorial and at least some explanation of how to tailor these generic instances to your needs.

The argonaut-codecs docs directory has a good example of expanded documentation for a Contributor library. But it would even be useful to add something considerably smaller and shorter to this library.

Additional context
See the Governance repository for more information about requirements in the Contributors organization.

Could the following two lines of code be switched around ?

Reason being that right now values come first in the encoded json and tag later. But for decoding it can potentially be faster to first get the tag to get of the type (FP) or class (OO) that the values have to be deserialized into.

Also for debugging it's nicer to see tag first and (potentially long) value string after that.

starting with v0.12 purescript-generics will no longer be supported by the compiler/deriving but purescript-generics-rep still is.

The later removed support for rows/records though and it seems RowToList is the way to go.

We need a way to deal with this in order for this lib to keep being usable after people switch to 0.12 or higher

This results in greater flexibility and smaller serialized data.