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

Default attribute should make the property required #153

Closed
sceccotti89 opened this issue Aug 1, 2023 · 5 comments
Closed

Default attribute should make the property required #153

sceccotti89 opened this issue Aug 1, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers question Further information is requested

Comments

@sceccotti89
Copy link

Currently a schema like this:

{
  type: "object",
  properties: {
    value: { type: "number", default: 5 },
  },
}

results in this type:

{
  [x: string]: unknown;
  value?: number | undefined;
}

I believe that a property with a defined default value should be treated as not optional, resulting in a type like the following:

{
  [x: string]: unknown;
  value: number;
}

What do you think?

@ThomasAribart
Copy link
Owner

Hmm I think it really depends on the context. Consider those two cases:

  • I handle validated value => value should be required
  • I input raw value (not validated) to a form or HTTP endpoint => value should be optional

So I think it should be configured through an option: FromSchema<mySchema, { someOption: boolean }>

The question now is what is default behavior of FromSchema ? I feel like it's more frequent to use FromSchema for validated values rather than inputs, so the option should be { keepDefaultedValuesOptional: true } ? That's a breaking change but a rather small one so I would be OK with implementing it.

WDYT ?

@ThomasAribart ThomasAribart added enhancement New feature or request good first issue Good for newcomers question Further information is requested labels Aug 2, 2023
@ThomasAribart ThomasAribart self-assigned this Aug 2, 2023
@sceccotti89
Copy link
Author

sceccotti89 commented Aug 2, 2023

I think it totally make sense.
I tried to do it myself but typings is somewhat a bit dark magic to me.
It would be great you can take care of that, but I'm also curious about the changes that you'll make to implement it.

@iamarthurk
Copy link

iamarthurk commented Nov 30, 2023

Much needed feature guys, let me know if it is on the roadmap or whether you are open to pull requests.

@ThomasAribart
Copy link
Owner

Feature is now available in the v3: #153 🙌

Wasn't so long to implement, sorry for the delay !

@davidmurdoch
Copy link

I feel like it's more frequent to use FromSchema for validated values rather than inputs

Interesting take. I think the use case of pre/post validation would be 50/50. The interface/function doing the validation of the raw data would need default-less types.

In my specific case I don't need to validate the data with defaults applied, since I'm calling into another API that will apply necessary defaults for me.

Of course this change doesn't really causes any issues, since the keepDefaultedPropertiesOptional option was also added, I just wanted to offer additional perspective on how this (awesome!) library is being used out in the wild. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants