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 native Elixir JSON support #394

Merged
merged 6 commits into from
Jan 13, 2025
Merged

Add native Elixir JSON support #394

merged 6 commits into from
Jan 13, 2025

Conversation

v0idpwn
Copy link
Collaborator

@v0idpwn v0idpwn commented Jan 10, 2025

This implementation defaults to JSON if it's available. If it's not,
it tries to use Jason as it did before.

Some exceptions are changed (only for JSON, Jason is unaffected). For
example, instead of Jason.DecodeError, when using JSON we raise
Protobuf.JSON.DecodeError, and encode errors have a more varied shape
(since they can be mostly anything coming from Elixir side).

Additionally, it adds 1.18.1 in CI. Once it's working in both versions, I'll
remove 1.17.x.

This implementation defaults to `JSON` if it's available. If it's not,
it tries to use `Jason` as it did before.

Some exceptions are changed (only for `JSON`, `Jason` is unaffected). For
example, instead of `Jason.DecodeError`, when using `JSON` we raise
`Protobuf.JSON.DecodeError`, and encode errors have a more varied shape
(since they can be mostly anything coming from Elixir side).
@v0idpwn v0idpwn marked this pull request as draft January 10, 2025 22:20
@v0idpwn v0idpwn marked this pull request as ready for review January 11, 2025 12:09
@v0idpwn
Copy link
Collaborator Author

v0idpwn commented Jan 11, 2025

If we don't want to default to it, we can make it configurable and keep the default as Jason. But I don't see good reasons for that.

lib/protobuf/json.ex Outdated Show resolved Hide resolved
@v0idpwn v0idpwn merged commit 0e6bbc5 into main Jan 13, 2025
4 checks passed
@v0idpwn v0idpwn deleted the feat/elixir-json-support branch January 13, 2025 02:05
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.

1 participant