Should %Group{} bindings return a map?

[mhanberg]

Currently, if you declare bindings in a group, they are stored as a keyword list. The ordering of this keyword list is not guaranteed (you might suspect that they would be in the order in which they are declared), because Vapor will internally coerce them into a map, and then coerces them back into a keyword list.

This was discovered when attempting to pattern match on the config.

You can see a basic demonstration of the failing behavior here

config = Vapor.load!([
  %Dotenv{},
  %Group{
      name: :endpoint,
    providers: [
      %Env{
        bindings: [
          {:url, "ENDPOINT_URL"},
          {:port, "ENDPOINT_PORT"},
          {:secret_key_base, "SECRET_KEY_BASE"}
        ]
      }
    ]
      }
    ])

[url: url, port: port, secret_key_base: secret_key_base] = config.endpoint

Even though we pattern match in the same order that we declared the bindings, it will raise a MatchError.

If we change the Group bindings implementation to create a map instead of a keyword list, the ergonomics slightly improve (IMO).

https://github.com/keathley/vapor/blob/ee360c7f23d9f15b697d6b6bf0e9b6f75a1bec9d/lib/vapor/providers/group.ex#L31

defimpl Vapor.Provider do
  def load(%{providers: providers, name: name}) do
    with {:ok, config} <- Vapor.Loader.load(providers) do
      {:ok, %{name => Map.new(config)}}
    end
  end
end

We could then pattern match on a map instead

config = Vapor.load!([
  %Dotenv{},
  %Group{
      name: :endpoint,
    providers: [
      %Env{
        bindings: [
          {:url, "ENDPOINT_URL"},
          {:port, "ENDPOINT_PORT"},
          {:secret_key_base, "SECRET_KEY_BASE"}
        ]
      }
    ]
      }
    ])

%{url: url, port: port, secret_key_base: secret_key_base} = config.endpoint

Let me know what you think.