Skip to content
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

Add value and decoded types and typedocs. #129

Closed

Conversation

azizk
Copy link
Contributor

@azizk azizk commented Jan 31, 2021

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. 👍

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.

@michalmuskala
Copy link
Owner

michalmuskala commented Jan 31, 2021

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.

@azizk
Copy link
Contributor Author

azizk commented Feb 1, 2021

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.

@azizk
Copy link
Contributor Author

azizk commented Feb 1, 2021

By the way, the term type includes pid, port, reference, function, BitString etc. Maybe it's worth it to restrict the encodable type somewhat?

@azizk azizk force-pushed the aziz/value-and-decoded-types branch from d9fb885 to 89a25f5 Compare June 13, 2021 18:14
@azizk
Copy link
Contributor Author

azizk commented Jun 13, 2021

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.

@azizk azizk force-pushed the aziz/value-and-decoded-types branch from 89a25f5 to e28cb34 Compare October 15, 2021 12:57
@azizk azizk force-pushed the aziz/value-and-decoded-types branch 2 times, most recently from 8061e9b to e79fcc4 Compare December 23, 2021 21:35
@azizk azizk force-pushed the aziz/value-and-decoded-types branch from e79fcc4 to e70764e Compare January 7, 2023 10:31
@azizk azizk force-pushed the aziz/value-and-decoded-types branch from e70764e to 3d38761 Compare October 15, 2023 21:34
azizk added 7 commits March 9, 2024 13:55
* Moved the plain JSON types to the top.
* Added type `str` to have a complete set of plain JSON types.
* Refer to options either as encoder or decoder options.
* Renamed some types to include a clarifying enc_/dec_ prefix.
@azizk azizk force-pushed the aziz/value-and-decoded-types branch from 3d38761 to 978b27b Compare March 9, 2024 12:55
@azizk
Copy link
Contributor Author

azizk commented Sep 17, 2024

Since there hasn't been any activity on this PR for a while, I'll close it for now. Feel free to use any changes in the future if they're helpful.

@azizk azizk closed this Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants