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

Include Types::Params::String #403

Closed
adamransom opened this issue Aug 4, 2020 · 8 comments
Closed

Include Types::Params::String #403

adamransom opened this issue Aug 4, 2020 · 8 comments

Comments

@adamransom
Copy link

adamransom commented Aug 4, 2020

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
Copy link
Member

solnic commented Aug 4, 2020

@adamransom should this be a coercible string?

@adamransom
Copy link
Author

adamransom commented Aug 4, 2020

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
Copy link
Member

solnic commented Aug 4, 2020

@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
Copy link
Author

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

@adamransom
Copy link
Author

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
Copy link
Member

solnic commented Aug 4, 2020

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

@adamransom
Copy link
Author

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
Copy link
Member

Using nominal type won't work, see #394 But I think strict types should work 🤔

flash-gordon added a commit that referenced this issue Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants