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

Add support for parameter defaults #8

Open
ezekg opened this issue Feb 12, 2025 · 4 comments
Open

Add support for parameter defaults #8

ezekg opened this issue Feb 12, 2025 · 4 comments

Comments

@ezekg
Copy link
Member

ezekg commented Feb 12, 2025

Might be useful for things like roles, pagination, etc.

typed_params {
  param :foo, type: :string, default: -> { 'bar' }
  param :baz, type: :string, default: 'qux'
}
def create = ...
@TheDauthi
Copy link

  • I'd use this in something I'm working on at this very second.

  • I started to implement this last year as "default adds a proc to the beginning of the transforms array that never changes the key and only changes the value if the value isn't empty?", which is a bit of a suboptimal way of doing it but seemed like it would touch the absolute fewest internals and let me see how my team used the feature. Basically how KeyAlias and such works, though.

  • I considered adding a callback to the types so a type can check to see if a value is a default instead of just checking empty? before rewriting it. My team came up with some cases in our existing code where it might be useful with custom types, but I ended up not being sold on it.

@ezekg
Copy link
Member Author

ezekg commented Feb 13, 2025

Thanks for chiming in, I really like the idea of making it a transform like key casing/alias. But we'd probably only want to apply the default if the param is missing, so that we don't clobber values, because an empty string and nil could be expected values for some. Not sure if a transform can achieve that, but worth looking into — may have to be its own Defaulter mapper in the processor pipeline, after transforms.

@TheDauthi
Copy link

That would be cleaner for running it before coerce, IIRC, which means that the final type info only lives in one place.

Whether actual transforms (like, from the transforms keyword, not other stuff in the array) should happen before or after... I could go either way, but do think that since the example in the README shows something being restructured (so that is an intended purpose), my gut is that it makes sense to be before, since otherwise you'd end up writing anything similar to that restructuring code twice.

I agree on empty? though, that's one of the main reasons I thought it was a suboptimal approach.

@ezekg
Copy link
Member Author

ezekg commented Feb 13, 2025

Yep, the problem with using transforms is that iirc the transforms only apply to parameters that are provided — i.e. you won't be able to provide a default for a missing parameter. That's why I was thinking a separate step in the pipeline, barring big changes. But maybe I'm wrong… I'll need to spend time reorienting myself with the internals to see if it's possible via the transformer:

module TypedParams
class Transformer < Mapper
def call(params)
depth_first_map(params) do |param|
schema = param.schema
parent = param.parent

I think we may be able to check if params is nil and apply a default via schema.default before we depth-first map.

Open to a PR if you're up for it, ofc.

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

2 participants