jason
jason copied to clipboard
Add value and decoded types and typedocs.
Hi!
I'd like to propose adding two types. Namely value
and decoded
. I sometimes noticed that it would be great to have a JSON value type handy, when I was dealing with JSON data. I'm sure others would benefit as well when they can refer to a canonical type from the most popular JSON Elixir library itself (e.g. @spec f(Jason.value()) :: Jason.value()
).
I gave the input parameters the decoded
type so that dialyzer may catch instances where developers unknowingly pass a tuple to the encode functions. But I'm not sure if that really helps or if it will be more of a hindrance.
I distinguish the value
type from the decoded
type, which can have any kind of map keys, so that we can have a stricter and more basic type when we're dealing with JSON data. :+1:
PS: You'll notice I changed String.t()
to binary
. It's a matter of preference to me and we can change it back to String.t()
of course.
Unfortunately using the decoded
type as input to the encode
functions is incorrect since any value implementing the Jason.Encoder
protocol can be passed - in fact you could implement it for tuples as well, it's just that it's not implemented by default (since there's no good default representation for tuples in JSON). You can't really do better than term
as input there. Additionally, Jason operates only over valid utf-8, which is signalled by using the String.t
type, rather than arbitrary binary
.
Oh I see. But do you think it would be a reasonable assumption to make that normally hardly anyone will implement the encoder protocol for tuples? I don't know, maybe in 1% of all cases someone might want to, but in those cases they probably need to bring the data into an encodable shape anyway before they pass it to Jason.encode()
. So perhaps it would be better to be on the safer side for the majority of usage cases. So maybe we should allow everything in the encode spec except for tuples?
In any case, what do you think about restricting the output type of the decode function? Looks good to me as far as I can see.
PS: I made some additional commits to improve things.
By the way, the term
type includes pid
, port
, reference
, function
, BitString
etc. Maybe it's worth it to restrict the encodable type somewhat?
Hey @michalmuskala, I changed the code so that encodable
is basically a term
again. I made some other changes too. I hope it's possible for you to review and sign it off this time? Appreciate your time.