dry-initializer icon indicating copy to clipboard operation
dry-initializer copied to clipboard

Incorrect processing of nil value for array with nested structure

Open StanisLove opened this issue 1 year ago • 3 comments

Describe the bug

I get a KeyError when pass the nil value for option, but it's ok when the option is omitted

To Reproduce

class X
  extend Dry::Initializer

  option :fields, [], optional: true, default: proc { [] } do
    option :name
  end
end

X.new #=> #<X:0x000055cce7f56888 @fields=[]>
X.new(fields: []) #=> #<X:0x000055cce7df0b10 @fields=[]>
X.new(fields: nil) #=> KeyError (X::Fields: option 'name' is required)

Expected behavior

Getting and empty array

My environment

  • Ruby version: 2.7.5
  • OS: Linux
  • dry-initializer (3.1.1)

StanisLove avatar Oct 24 '24 04:10 StanisLove

And if I set the name as optional it doesn't make it much better...

class X
  extend Dry::Initializer

  option :fields, [], optional: true, default: proc { [] } do
    option :name, optional: true
  end
end

X.new(fields: nil) #=> #<X:0x000055cce6fe4760 @fields=[#<X::Fields:0x000055cce6fe45f8 @name=Dry::Initializer::UNDEFINED>]>

StanisLove avatar Oct 24 '24 04:10 StanisLove

Hi @StanisLove,

Thank you for opening this issue and providing clear examples. I’ve reviewed the code snippets, but I wasn’t able to locate the relevant code in the project that handles the fields option. As a result, I can't confirm the implementation details leading to the KeyError when passing nil.

From your description, it seems we should receive an empty array instead of an error. I also noticed that setting name as optional still results in unexpected behavior.

Is there any ongoing discussion or documentation about this feature? If not, I would suggest we revisit the handling of nil values for the fields option to ensure it defaults correctly when nil is passed.

If there’s no one currently working on a fix, I’d be happy to assist in implementing a solution once we clarify the intended behavior. Please let me know how you would like to proceed!

Thanks!

Risto752 avatar Oct 24 '24 13:10 Risto752

Using extend Dry::Initializer[undefined: false] instead should do the trick:

class X
  extend Dry::Initializer

  option :fields, [], optional: true, default: proc { [] } do
    option :name
  end
end

X.new(fields: nil).fields
#=> []

Drowze avatar Dec 11 '24 21:12 Drowze