wojtekmach / req

Req is a batteries-included HTTP client for Elixir.
https://hexdocs.pm/req
Apache License 2.0
1.07k stars 112 forks source link

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

Open zachallaun opened 2 months ago

zachallaun commented 2 months ago

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:

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

zachallaun commented 2 months ago

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

Does that sound about right?