Use the built-in JSON library#6481
Conversation
| end | ||
|
|
||
| assert_raise json_error, | ||
| ~r/invalid byte 111 at position \(byte offset\) 0|unexpected byte at position 0: 0x6F \("o"\)/, fn -> |
|
Looks good to me! But if we actually change new projects to use JSON, we'll also need to bump the minimum version we set to Elixir 1.18, see Lines 12 to 20 in 9fc21c6 I wonder if we should postpone this change to a future Phoenix 1.9? @josevalim |
josevalim
left a comment
There was a problem hiding this comment.
Yes, v1.9 material for sure. But it looks good. We can probably assign a v1.9 milestone and merge it once we finally branch v1.8 out. I don’t think it is worth branching out though, as I dont think there is anything beyond this PR for now?
I don't think there's anything else I can add to this PR. Thank you everyone for looking into this! 🙇 |
|
Looking at this PR - I believe it's not flexible enough to allow support of different JSON libraries beyond |
|
I’d recommend modules to provide a similar interface to the JSON in Elixir, as that will become effectively standard over time. And most of it was mirrored after Jason anyway for compatibility. |
|
What would be a downside of providing a JSON behavior for phoenix? |
|
The issue is that Ecto has its own library, Ash as well, and so on. If each of them need a behavior, you would then need all of them to agree or you end up with several wrappers. And if they are going to agree on one, the Elixir interface will most likely be the one to choose. |
|
What I am asking is to document it in some form (could be at the Elixir level) so that library developers could follow the spec. Right now there's none, which opens the doors for undesirably having to deal with several JSON library dependencies in your project (because Ecto, Phoenix, Ash, Bandit, etc. have their own API), when all you want is to have one pluggable JSON library of your choice. Having a JSON behavior at the Elixir level could help, but even if it's just formally defined in documentation, we'd be at a better place. |
|
@saleyn then send a PR please that adds a sentence to https://phoenix.hexdocs.pm/Phoenix.html#json_library/0, e.g.
|
I will submit a PR. @SteffenDE, I know you would probably hate this, but I still have to ask. Would it be possible to require that the canonical function names for the JSON library interface not end with '!', so that the Erlang libraries could be used without requiring them to have an Elixir wrapper? |
|
@saleyn if we change the API, then all existing uses of JSON and Jason then become broken, no? |
This highlights a problem that was created when JSON's interface was designed. I guess the design didn't consider that it would become the standard for cross-languages. Maybe it can be extended with adding On the other hand, I was under the impression that Erlang used to not permit '!' in function names, but I just tried it, and it does seem to work: $ cat mod.erl
-module(mod).
-export(['f!'/0]).
'f!'() ->
os:system_time().
$ erlc mod.erl
$ erl
Erlang/OTP 29 [erts-17.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]
Eshell V17.0 (press Ctrl+G to abort, type help(). for help)
1> mod:'f!'().
1781527995879131559So, my comment about the need to change the API is moot. @josevalim are there any other concerns for an Erlang library to expose |
|
Yes, the design happened by accident, unfortunately. And yes, you can totally have |
| """ | ||
| def json_library do | ||
| Application.get_env(:phoenix, :json_library, Jason) | ||
| Application.get_env(:phoenix, :json_library, @default_json_library) |
There was a problem hiding this comment.
Consider :persistent_term, which is roughly 30-100% faster:
defmodule JSON.Adapter do
@default_json_library if Code.ensure_loaded?(JSON), do: JSON, else: Jason
def json_library do
case :persistent_term.get(:json_library, nil) do
nil ->
module = :application.get_env(:phoenix, :json_library, @default_json_library)
:persistent_term.put(:json_library, module)
module
module ->
module
end
end
endThere was a problem hiding this comment.
TIL persistent_term and it looks cool, but I'm not sure if we are in the position of adding anything significant to this PR. I'm happy to add it, but it's @SteffenDE call at this point
rhcarvalho
left a comment
There was a problem hiding this comment.
In the meantime HexDocs URLs now use subdomains.
The installer could use runtime detection to decide on the JSON library for new apps, that will keep the installer compatible with Elixir pre-1.18.
| # Use Jason for JSON parsing in Phoenix | ||
| config :phoenix, :json_library, Jason | ||
| # Use built-in `JSON` for JSON parsing in Phoenix | ||
| config :phoenix, :json_library, JSON |
There was a problem hiding this comment.
I think here we could do a similar runtime detection as in https://github.com/phoenixframework/phoenix/pull/6481/changes#r2356177940, and only configure JSON as the default for Elixir versions where its available.
Same for the umbrella app config template.
There was a problem hiding this comment.
It's a nice idea, yet I don't think it would fly.
The first option is to check Elixir version during phx.new, but the Elixir version that generates the project is not guaranteed to match the Elixir version that runs the project. So it sounds like a breaking change.
The second option is to embed this check into new projects. Technically it should work, but I'm not sure that such checks are awkward and probably are not something you would like to see in new projects.
Co-authored-by: Rodolfo Carvalho <rhcarvalho@gmail.com>
Co-authored-by: Rodolfo Carvalho <rhcarvalho@gmail.com>
@SteffenDE, could you clarify if this was a typo - there's no |
yes! |
Export 'decode!'/1, 'encode!'/1, and 'encode_to_iodata!'/1 from glazer_json as thin aliases for decode/1 and encode/1, matching the pluggable json_library() interface Elixir 1.20.x / Phoenix expect (phoenixframework/phoenix#6481). Document this compliance in the README. Also add update_reduction_count() in glazer_common.hpp, which calls enif_consume_timeslice for inline (sub-DIRTY_THRESHOLD) NIF calls so the BEAM's reduction accounting reflects real work done instead of charging a flat 1 reduction per call. Wired into the decode/encode/scan/minify/prettify NIFs for JSON, YAML, and CSV.
|
Please check my PR above (#6732). As a reference implementation, the glazer_json module (part of the glazer Erlang JSON library) exposes these three functions under their quoted Erlang names — 'decode!'/1, 'encode!'/1, and 'encode_to_iodata!'/1 — and can be configured directly: config :phoenix, :json_library, :glazer_json For parity with Elixir's JSON module, where null decodes to and encodes from nil rather than the atom :null, these three functions apply that mapping automatically. |

Address #6461
The changes set the built-in JSON library as a default JSON library.
The idea is to check whether the built-in JSON library is present and use it if so:
Correct me if I'm wrong, but I think a naive check of whether the library is present during compilation time should work, as it's a built-in library.
What I'm not sure about is whether such a move results in a breaking change.
It could be a breaking change for projects that don't specify the
json_libraryand use@derive Jason.Encoderon structures passed toPhoenix.Controller.json/2. That sounds pretty niche, especially given that the template project includes:Also, I'm specifically leaving the
jasondependency in place for pre-27 OTP versions.Please let me know what you think and whether the changes make sense. In other words, maybe it would be better to stick with
Jasonuntil Phoenix requires OTP 28.Similar discussions / changes:
Use Jason instead of Poison for json encoding by chrismccord #2734phoenixframework/phoenix
Investigate using Jason instead of Poison #2693phoenixframework/phoenix
Use JSON instead of Jason by kubosuke #234felt/geo