Skip to content
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

Types.Constructor failing on Ruby 3.1 #459

Open
danillos opened this issue Jul 25, 2023 · 8 comments
Open

Types.Constructor failing on Ruby 3.1 #459

danillos opened this issue Jul 25, 2023 · 8 comments

Comments

@danillos
Copy link

Describe the bug

Make a way to Types.Constructor works without a Hack.

This issue was copied from https://discourse.dry-rb.org/t/types-constructor-failing-on-ruby-3-1/1566 and originally created by eaylward8

To Reproduce

Given this class:

class User
  def initialize(name:)
    @name = name
  end
end

…the docs say I should be able to do this:

user_type = Types.Constructor(User)

# It is equivalent to User.new(name: 'John')
user_type[name: 'John']

On Ruby 2.7.7, this works as expected and I get an instance of User with the given name.

On Ruby 3.1.2, I get:

Dry::Types::CoercionError: wrong number of arguments (given 1, expected 0; required keyword: name)
Caused by ArgumentError: wrong number of arguments (given 1, expected 0; required keyword: name)
from (pry):2:in `initialize'

Expected behavior

Doesn't raises an error like the Doc says.

My environment

  • Affects my production application: NO
  • Ruby version: ruby 3.1.2p20
  • OS: MacOS
@flash-gordon
Copy link
Member

In this case, the docs should be updated. Thanks for filing the issue

@flash-gordon
Copy link
Member

Can you point out where you found this class definition?

class User
  def initialize(name:)
    @name = name
  end
end

@danillos
Copy link
Author

Can you point out where you found this class definition?

class User
  def initialize(name:)
    @name = name
  end
end

The docs don't have it there but based on the example here, I think it is the User definition.

@danillos
Copy link
Author

danillos commented Nov 27, 2023

I did it as a temporary fix:

module Types
  include Dry::Types(default: :strict)

  def self.Constructor(klass)
    # DryTypes Constructor doesn't support Ruby 3.1 initialization
    # this is temporary code to make it work.
    #
    # See more details at: https://github.com/dry-rb/dry-types/issues/459
    super { |item| klass.new(**item) }
  end
end

@flash-gordon
Copy link
Member

The docs don't have it there but based on the example here, I think it is the User definition.

This assumption is wrong, Types.Constructor expects sequential arguments. The correct definition is

class User
  def initialize(attributes)
    @attributes = attributes
  end

  def name = @attributes.fetch(:name)
end

We should update the docs with a sample User definition.

@flash-gordon
Copy link
Member

I did it as a temporary fix:

I would suggest using a different name

module Types
  include Dry::Types(default: :strict)

  def self.KwConstructor(klass)
    # DryTypes Constructor doesn't support Ruby 3.1 initialization
    # this is temporary code to make it work.
    #
    # See more details at: https://github.com/dry-rb/dry-types/issues/459
    super { |item| klass.new(**item) }
  end
end

but it's up to you.

@danillos
Copy link
Author

danillos commented Nov 28, 2023

The docs don't have it there but based on the example here, I think it is the User definition.

This assumption is wrong, Types.Constructor expects sequential arguments. The correct definition is

class User
  def initialize(attributes)
    @attributes = attributes
  end

  def name = @attributes.fetch(:name)
end

We should update the docs with a sample User definition.

Hi @flash-gordon,

Thank you for your attention,

My case wasn't related to this User example, this example I copied from the original reported issue here

My case was using Dry Initializer like:

  class User
    extend Dry::Initializer[undefined: false]
    option :id, type: Types::Integer
  end

  class Delete
    extend Dry::Initializer[undefined: false]
    option :user, type: Types.Constructor(::User)
  end

  # and using it like:

  Delete.new(user: { id: 1 })
  
  # `block in __dry_initializer_initialize__': User: option 'id' is required (KeyError)

So using only Dry Gems and only in Ruby 3.1+, I'm having this issue, what is weird. It seems like a bug. Since everyone using Ruby before 3.1 on this way, will have this issue when upgrading to Ruby 3.1

@flash-gordon
Copy link
Member

It's because dry-initializer uses keywords. Before 3.0 ruby was more permissive regarding passing hashes where keywords are expected. Once a clear separation was made in 3.0 it doesn't work magically anymore. I cannot see what dry-types or dry-initializer can/should do about that. Defining a custom constructor is a consistent and non-breaking solution.

To be honest, I'm trying to stay out of the subject, it is very-very-very likely if we dig deeper by trying to fix anything here we'll come up with more bugs. Also, performance may suffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants