Skip to content

config/3 re-orders keyword lists while deep-merging #14461

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

Closed
florius0 opened this issue May 1, 2025 · 1 comment
Closed

config/3 re-orders keyword lists while deep-merging #14461

florius0 opened this issue May 1, 2025 · 1 comment

Comments

@florius0
Copy link

florius0 commented May 1, 2025

Elixir and Erlang/OTP versions

Erlang/OTP 27 [erts-15.2.3] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [jit]

Elixir 1.18.3 (compiled with Erlang/OTP 27)

Operating system

Darwin MacBook-Pro 24.4.0 Darwin Kernel Version 24.4.0: Fri Apr 11 18:32:50 PDT 2025; root:xnu-11417.101.15~117/RELEASE_ARM64_T6041 arm64

Current behavior

Consider the following config.exs

import Config

config :config_reorder_reproduction, :key,
  a: 1,
  b: 2,
  c: 3

config :config_reorder_reproduction, :key,
  a: 2,
  b: 1

Application.get_env(:config_reorder_reproduction, :key) returns [c: 3, a: 2, b: 1].

Expected behavior

From my point of view, the Application.get_env(:config_reorder_reproduction, :key) should return [a:2, b: 1, c: 3, since in the first config/3 they were configured in that particular order, and keyword-lists are ordered.

Use case

When configuring something similar to pipeline, .e.g:

# config/config.exs
import Config

config :my_app, MyApp.Pipline1, [
    {MyApp.Pipline1.Step1, ...},
    ...
    {MyApp.Pipline1.StepN, ...},
  ]

# config/runtime.exs

config :my_app, MyApp.Pipline1, [
    {MyApp.Pipline1.Step1, secret: System.fetch_env!("PIPELINE_1_STEP_1_SECRET")}
  ]

it is convenient only to configure secrets for the steps in runtime.exs, and not copy-paste the whole pipeline configuration, but currently the bug breaks the order on which MyApp.Pipline1 may actually rely.

PS

I suppose one could write the steps explicitly and then have only the values of the particular key/step be merged, but it would be better to have this behaviour documented.

# config/runtime.exs

config :my_app, MyApp.Pipline1, [
    {MyApp.Pipline1.Step1, secret: System.fetch_env!("PIPELINE_1_STEP_1_SECRET")}
    ....
    {MyApp.Pipline1.StepN, []}
  ]
@josevalim
Copy link
Member

The current behaviour is the behaviour of Keyword.merge, which basically concatenates the second list into the first one. So you get:

first_list_minus_elements_from_second_list ++ second_list

I think your proposal can be problematic in cases like this:

iex(1)> Keyword.merge([a: 1, b: 2], [b: 1, a: 2])

According to your proposal, it would return [a: 2, b: 1] but I would say the order in the second list should prevail (because the second list should win).

You could argue that we should change Keyword.merge so the response should be this:

second_list ++ first_list_minus_elements_from_second_list

Which would fix your example but it would not return what you expect for merge([a: 1, b: 2, c: 3], [b: 5, c: 6]) (as a: 1 would now come at the end).

So overall, I think trying to get any sort of ordering guarantee from this would have pitfalls. My suggestion is to wrap the pipeline into a tuple or similar, so you always fully replace it.

@josevalim josevalim closed this as not planned Won't fix, can't repro, duplicate, stale May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants