dashbitco / nimble_options

A tiny library for validating and documenting high-level options. 💽
Apache License 2.0
507 stars 38 forks source link

Proposal: `NimbleOptions.derive_struct` and `into` option. #124

Closed zachdaniel closed 8 months ago

zachdaniel commented 8 months ago

A big quality of life improvement for our usage of NimbleOptions would be the following:

  1. The ability to derive a documented struct from options.
  2. The ability to validate "into" a struct.
  3. The ability to specify the into option for nested keyword lists

An example would be something like:

defmodule Discombobulator do
  require NimbleOptions

  @whirligig_schema [...]

  NimbleOptions.define_struct(WhirligigOptions, @whirligig_schema)

  @schema [
    times: [
      type "How many times to frobulate"
    ],
    distance: [
      type: "How far to reticulate the splines"
    ],
    whirligigs: [
      type: :keyword,
      schema: @whirligig_schema,
      into: WhirligigOptions
    ]
  ]

  NimbleOptions.define_struct(Options, @schema)

  def discombobulate(opts) do
    case NimbleOptions.validate(opts, @schema, into: Options) do
      {:ok, %Options{}} -> ...
      {:error, error} -> ...
   end
  end
end

The benefit of having options specified as a struct is that you don't need to traverse the options list many times to lookup keys (small but tangible for options validated/accessed often), and you can get editor/language server hints on your options.

I think we could get a lot of the benefits without #1, where we require that the structs be hand written, but then you do end up needing to duplicate the keys and do up the typespecs yourself, that kind of thing.

Thoughts?

whatyouhide commented 8 months ago

@zachdaniel we're really trying hard to keep this nimble, so let's see if there's a way around this. My first instinct is:

@schema [
  times: ...,
  distance: ...
]

defstruct Keyword.keys(@schema)

def discombo(opts) do
  opts |> NimbleOptions.validate!() |> then(&struct!(__MODULE__, &1))
end

Would something like this work? The only downside with this is that you still do two passes over the kw list, one for validating and another one in struct!/2, but my experience is that you validate once (or rarely) and access many times. Thoughts?

zachdaniel commented 8 months ago

Yep! I was pretty sure you'd prefer not to have the struct derivation bit 😆 I can also do that in my own helper if I want. I think it works well enough to do it that way. I'm wondering if there is a way to use the type spec tooling to define the type spec for the struct as well. It gets problematic with nested schemas, i.e I'd need to delete the nested structs, and then define a separate struct/typespec for that struct.

zachdaniel commented 8 months ago

I think the into option would be pretty lightweight, though. You could provide a map, struct or keyword list. Has many uses, but is simple in operation and to understand. i.e

.validate(..., into: existing_opts)
whatyouhide commented 8 months ago

It gets problematic with nested schemas

Yes this is one of my main reasons to avoid adding this, because it's something we then might need to support in the library too if we accept define_struct/2.

:into might be okay, but then again, struct/2 already does that, so I’m not sure we're adding much value?

zachdaniel commented 8 months ago

👍 I can always make a helper that has the interface I like, but the fact that its not possible to do it w/o a second iteration is the part that I'm not a big fan of personally. We use nimble options all over the place in Ash framework, in some places that are called very often, so the into option could be a non-trivial optimization for us.

But if its not right for nimble_options, its no worries :)

I'll close this issue for now, thanks for entertaining the idea!

whatyouhide commented 8 months ago

@zachdaniel btw, in a library as big and complex as Ash, I'd lean on rolling out your own option-validation thing. nimble_options is really really tiny, you could just copy the source into Ash and go from there 😄

zachdaniel commented 8 months ago

Our work with nimble_options is primarily done through spark: https://hex.pm/packages/spark So that is where we would end up doing this, with a module like Spark.Options.

Part of our goals for 2024 is to make sure that we are contributing as much as possible to the libraries we use and to the Elixir ecosystem, so I wanted to check here before I did exactly that :)

whatyouhide commented 8 months ago

@zachdaniel oh yeah don't get me wrong this was a good discussion and we absolutely appreciate the suggestion 🙃