dry-rb / dry-types

Flexible type system for Ruby with coercions and constraints
https://dry-rb.org/gems/dry-types
MIT License
861 stars 134 forks source link

Include `Types::Params::String` #403

Closed adamransom closed 3 years ago

adamransom commented 4 years ago

When including types using params as the default

module Types
  include Dry.Types(default: :params)
end

for use in form objects for example, it would be nice to be able to define an attribute in a struct as being a regular string

class MyStruct < Dry::Struct
  attribute :some_integer_param, Types::Integer
  attribute :name, Types::String
end

Currently this doesn't work and you instead need to define the name attribute with Types::Nominal::String which just feels a bit off.

I currently work around this by just defining my own String inside the Types module, but I was just wondering whether it wouldn't make sense inside the Params namespace.

solnic commented 4 years ago

@adamransom should this be a coercible string?

adamransom commented 4 years ago

Well, a string is a string I guess 😄 but I see the Params namespace as a place for all things you would usually expect to receive from HTTP parameters, String being one of them.

Though perhaps it is purely meant for coercion of things from params, in which case I suppose my work-around is what's intended?

But yeah, this issue is mainly just an ergonomics thing when using default: :params.

solnic commented 4 years ago

@adamransom I think it should be added as Types::Params::String = Types::Nominal::String because param values are either string, array or hash and we don't want to accidentely coerce an array or a hash into a string. Does this make sense?

adamransom commented 4 years ago

Yeah, definitely agree with that, and that's what I use as a workaround for now.

adamransom commented 4 years ago

Though I suppose if we go down this route, we'd need to add ones for array and hash too, in order to be consistent with Params having everything we'd expect from HTTP params.

solnic commented 4 years ago

@adamransom good point. This also should be documented so that people don't expect any type of coercion.

adamransom commented 4 years ago

Right, I guess the short description is:

Params contains every type you would expect to receive via HTTP params, coercing where it makes sense. Currently, everything is coerced except Params::String, Params::Array and Params::Hash.

Would we want to use Nominal or Strict as the types behind Params::String, Params::Array and Params::Hash though? The params type raise if they can't be coerced, so I'd imagine the expectation is to raise if the type isn't an array, for example.

flash-gordon commented 4 years ago

Using nominal type won't work, see https://github.com/dry-rb/dry-types/pull/394 But I think strict types should work 🤔