dry-rb/dry-initializer

Option not properly working when combine with dry-container

gottfrois opened this issue · 1 comments

module Container
  extend Dry::Container::Mixin

  register(:deps) {}
end

module Foo
  Import = Dry::AutoInject(Container)
end

class SomeService
  extend Dry::Initializer
  include Foo::Import['deps']

  option :foo
end

Then try to instantiate the service with:

SomeService.new(foo: 1)
KeyError: SomeService: option 'foo' is required
from (eval):4:in `block in __dry_initializer_initialize__'

I'm using dry-initializer (2.3.0)

Hi, @gottfrois! Thank you for the report

The root of the problem is in a way kwargs strategy of dry-auto_inject decides how to merge container to given arguments. Namely, it backs on the presence of :keyrest part in the original signature of the initializer.

But Dry::Initializer::Mixin::Root defines the signature of the #initializer as def initializer(*args).

That's why kwargs strategy ultimately reloads the initializer as:

def initialize(*args, **kwargs)
  super(*args, )
  # ...
end

It drops out the last argument (which I think is a bug per se, because it removes the last argument even when it cannot merge a content of a container to it. I believe, in that case the original data would be kept intact).

The possible solution is not obvious.

I cannot simply update the signature in a dry-initializer like this:

def initialize(*args, **opts)
  __dry_initializer_initialize__(*args, **opts)
end

because this would break an important distiction between two cases where the last hash should be treated either as a param, or a bunch of options, depending on the definition:

class Foo
  extend Dry::Initializer
  param :foo
  param :bar
end

class Bar
  extend Dry::Initializer
  param  :foo
  option :baz
end

I'd bet it is kwargs that shouldn't back on a concrete implementation of the signature of the original initializer.

For example, it could check an actual content of the last argument, and decide whether it should be merged to a container. But this could be done for a cost of worse performance (more checks in a runtime).

@timriley @solnik Hi, guys, wdyt about this?