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

json: Support optional use of :json in OTP 27+ #386

Open
zachallaun opened this issue Jul 16, 2024 · 4 comments
Open

json: Support optional use of :json in OTP 27+ #386

zachallaun opened this issue Jul 16, 2024 · 4 comments

Comments

@zachallaun
Copy link
Contributor

With the release of :json in OTP 27, I think it's worth discussing how Req might support its (optional) use. I don't feel strongly that Req should support it (more on that below), but I figured a discussion would be worth having that could at least be pointed to in the future when the topic comes up!

Some points for context:

  • As of v0.5.2, Jason is one of Req's three required dependencies, alongside Finch and MIME. (Other de/encoding deps, such as NimbleCSV, brotli, et al. are optional.)
  • Req uses the following from Jason, not all of which have a direct correspondence in :json:
    • Jason.encode_to_iodata!/1 (opts not supported) - corresponds to :json.encode/1 (returns iodata and raises on error)
    • Jason.encode!/1 (opts not supported) - equivalent to value |> :json.encode() |> IO.iodata_to_binary()
    • Jason.decode/2 (opts supported with :decode_json) - no direct correspondence, :json.decode/3 raises on error and does not support high-level opts like keys: :atoms, but requires that the same effect be implemented using callbacks
  • One potentially critical component missing from :json is the protocol support that Jason provides for struct encoding. Whereas Jason will error when attempting to encode a struct that doesn't implement Jason.Encoder, :json will happily encode to {"__struct__": "Elixir.MyStruct", ...}.

Given that :json is not a drop-in replacement for Jason, I see a few ways forward:

  1. Do nothing and wait. There's an issue in the Jason repo (michalmuskala/jason#185) suggesting that the current plan is to propose an Elixir standard library wrapper around :json. We could wait for that and continue to depend on Jason. (It's possible that this wrapper, if accepted, would not be implemented until Elixir drops support for OTP 26, which I believe is 1.19 at the earliest.)
  2. Put the burden of using :json on the user: make Jason an optional dependency and add JSON encoder/decoder options that default to using Jason if present or raise an error suggesting to add the dependency otherwise.
    • :decode_json options could be passed to the decoder function; it would be up to the user to implement them in terms of :json if they've overridden the default Jason decoder.
  3. Put the burden of using :json on Req: make Jason an optional dependency and provide encoder/decoder implementations in order of Jason > :json > raise an error.
    • Would have to decide what to do about :decode_json options. I don't think it's reasonable for a Req-provided :json implementation to accept every option that Jason does.
    • Would have to decide what to do about the differences between Jason and :json when it comes to struct encoding.

This ended up a bit longer than I anticipated, but hopefully is a decent jumping-off point.

@wojtekmach
Copy link
Owner

Thanks for this! I think protocols is the deal breaker for now. This is possible today

Req.post!(url, json: %{now: Time.utc_now()})

and there's no replacement until Elixir gets JSON with protocols. At that point, it would be a breaking change too, instead of using Jason.Encoder we'd use JSON.Encoder, but I'm OK with that and I'll document it on Req v1.0. Even if Jason becomes a tiny shim over Elixir's JSON, I'd rather not depend on it anyway.

@wojtekmach
Copy link
Owner

regarding :decode_json option today, I can deprecate it in favour of passing, say, decode_json: &:json.decode(&1, ...), so that's not a big deal.

@wojtekmach
Copy link
Owner

wojtekmach commented Jul 16, 2024

Or, decoders: [json: &:json.decode(&1, ...)]. So yeah, we're good, we have long-term backwards compatible options.

@zachallaun
Copy link
Contributor Author

Okay, so to clarify the "plan" a bit:

  • We'll keep the Jason dependency until Elixir gets a JSON module w/ protocols
  • In the interim, we could deprecate :decode_json opts in favor of a :decode_json function (and presumably add an :encode_json as well?), which could allow using :json if desired

Does that sound about right?

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

No branches or pull requests

2 participants