elixir-cldr / cldr

Elixir implementation of CLDR/ICU
Other
440 stars 33 forks source link

Setting a json_library setting within a umbrella triggers Could not load configured :json_library #204

Closed jnylen closed 1 year ago

jnylen commented 1 year ago

Hi,

I have an umbrella set up like this:

The json library im trying to use is https://hex.pm/packages/jsonrs with this in the config:

config :sentry, :json_library, Jsonrs
config :postgrex, :json_library, Jsonrs
config :phoenix, :json_library, Jsonrs
config :phoenix, :format_encoders, "json-api": Jsonrs
config :ex_cldr, json_library: Jsonrs

It works for every library except ex_cldr which triggers this error:

Erlang/OTP 26 [erts-14.0] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit:ns]

==> ex_cldr
Compiling 2 files (.erl)
Compiling 42 files (.ex)

== Compilation error in file lib/cldr/config/config.ex ==
** (ArgumentError) Could not load configured :json_library, make sure Jsonrs is listed as a dependency
    lib/cldr/config/config.ex:151: (module)
could not compile dependency :ex_cldr, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile ex_cldr --force", update it with "mix deps.update ex_cldr" or clean it with "mix deps.clean ex_cldr"

The jsonrs is added as a dependency of the business application and both api and web is depended on the business app.

Tried adding it as a dependency of the other two apps but still triggered that issue, and adding it as an dependency in the main umbrella mix file. Still the same issue.

Any clues?

Right now i'm back at using Jason just for this library.

kipcole9 commented 1 year ago

That's definitely not a good outcome, thanks for the report. I've done some initial testing in a non-umbrella test app. I was getting the same symptoms are you describe when I compiled without ex_cldr_* as a dependency then added it as a dependency.

When I ran mix deps.clean --all && deps.get and it compiled correctly. I know its not convenient but would you mind trying that so we can eliminate it as a cause?

Part of the challenge is that ex_cldr_* use the :json_library at compile time (not runtime). Therefore :jsonrs needs to compile first and then be loadable during compilation. The steps above did produce the right dependency graph and compilation was correct.

Let me know if you see any different symptoms after trying the mix deps.clean --all && deps.get step?

kipcole9 commented 1 year ago

Would you also mind letting my know which ex_cldr_* libraries you are configuring - I want to make sure my test app is as close as possible to your configuration.

jnylen commented 1 year ago

Hi @kipcole9,

Yeah running that command works.

The libraries I use are these:

      {:ex_cldr, "~> 2.37"},
      {:ex_cldr_plugs, "~> 1.3"},
kipcole9 commented 1 year ago

Happy to hear that worked. I wish I could provide a more definitive reason why, when config changes, that cleaning deps is required. Only rarely, but enough times to be a pain. As far as I can tell this is only triggered on some configuration changes. I believe you should be ok on normal builds otherwise.

kipcole9 commented 1 year ago

BTW, ex_cldr will detect the json library used by Phoenix (or Ecto) so you shouldn't actually have to even configure one for ex_cldr in your configuration. Basically is looks like this:

poison = if(Code.ensure_loaded?(Poison), do: Poison, else: nil)
jason = if(Code.ensure_loaded?(Jason), do: Jason, else: nil)
phoenix_json = Application.compile_env(:phoenix, :json_library)
ecto_json = Application.compile_env(:ecto, :json_library)
cldr_json = Application.compile_env(:ex_cldr, :json_library)

@json_lib cldr_json || phoenix_json || ecto_json || jason || poison
kipcole9 commented 1 year ago

Closing for now given the resolution above. Please reopen if you think that is not appropriate!