search

garyb / purescript-codec-argonaut

Bi-directional JSON codecs for argonaut
MIT License
39 stars 16 forks source link

Add propOptional #61

Closed ysangkok closed 6 months ago

ysangkok commented 6 months ago

This fixes #60. We have been running this in production for about two months.

I put this in the Compat module because I wanted the Nothing encoded as null.

BTW I think it is really confusing that there are two maybe codecs in this library... Would be better to use distinct names. Not because I import things unqualified but because people might not realize there are two.

ysangkok commented 6 months ago

@garyb Any chance of a review for this?

garyb commented 6 months ago

I think I'm going to reject adding this to the library - I have some qualms about its codec-ness.

It is in the compat module, so is perhaps acceptable that it's not a real codec in regard to Maybe encoding (in that it can't encode an optional Maybe or a Maybe Maybe), but the way the NothingEncoding adds another layer of encoding/decoding disagreement on top.

I think it's fine to do this kind of thing when you know you're not going to fall into either of those traps, but I'd prefer to keep it local to a project that makes those assumptions rather than putting it in the library for all to use.

Thanks though!

ysangkok commented 6 months ago

I can see why you'd want the laws to be satisfied, but I don't understand why the laws are more important for the prop* set of methods than for the Compat.maybe codec, which also breaks the laws, as explained in the comment: Argonaut/Compat.purs line 24.

The existence of this function seems to recognize that people have a need to this (both nulls and omitting keys are common in practice). Why is propOptional held to a higher standard than Compat.maybe?