req icon indicating copy to clipboard operation
req copied to clipboard

`json`: Support optional use of `:json` in OTP 27+

Open zachallaun opened this issue 1 year ago • 4 comments

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.

zachallaun avatar Jul 16 '24 13:07 zachallaun

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 avatar Jul 16 '24 14:07 wojtekmach

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 avatar Jul 16 '24 14:07 wojtekmach

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

wojtekmach avatar Jul 16 '24 14:07 wojtekmach

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?

zachallaun avatar Jul 16 '24 15:07 zachallaun

+1 This would allow to have less extra dependencies

syepes avatar Mar 16 '25 11:03 syepes

Since Elixir now has a JSON module which is heavily inspired by Jason, I think it would make sense to add support for it. #441 mentioned breaking json_options, so I assume the only way is to make Jason an optional dependency and then pass the used library somewhere in the config (or just make a breaking release - best option IMO, but I understand if you don't want to do that yet)

flexagoon avatar Apr 15 '25 20:04 flexagoon

Looking at the source code it seems Req only calls Jason modules in three places (apart from some calls on tests): https://github.com/wojtekmach/req/blob/ffd3b9a2f6c845c3d345803627f8217f0917a134/lib/req/response.ex#L110 https://github.com/wojtekmach/req/blob/ffd3b9a2f6c845c3d345803627f8217f0917a134/lib/req/steps.ex#L469 https://github.com/wojtekmach/req/blob/ffd3b9a2f6c845c3d345803627f8217f0917a134/lib/req/test.ex#L206

So maybe we could do like oban did: https://github.com/oban-bg/oban/blob/780db5aeb4063989a7c1af2d50005efcfbe1ab28/lib/oban/json.ex.

Although here we return Jason.DecodeError https://github.com/wojtekmach/req/blob/ffd3b9a2f6c845c3d345803627f8217f0917a134/CHANGELOG.md?plain=1#L311

But JSON doesn't have a specific error for this:

iex(1)> JSON.decode("bla")
{:error, {:invalid_byte, 0, 98}}

So we'd have to create a custom Req error to return and it would be a breaking change unfortunately.

michelboaventura avatar May 08 '25 14:05 michelboaventura

Yeah, for Req v1.0 I plan to require Elixir 1.18 and default to Elixir JSON and for this we would get {:error, %JSON.DecodeError{}}.

wojtekmach avatar May 08 '25 14:05 wojtekmach

Amazing! Is there some place we can follow the path toward req in terms of what is done or is pending?

michelboaventura avatar May 08 '25 15:05 michelboaventura