ThomasAribart / json-schema-to-ts

Infer TS types from JSON schemas 📝
MIT License
1.43k stars 30 forks source link

Default attribute should make the property required #153

Closed sceccotti89 closed 8 months ago

sceccotti89 commented 1 year ago

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 commented 1 year ago

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

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 ?

sceccotti89 commented 1 year ago

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 commented 8 months ago

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

ThomasAribart commented 8 months ago

Feature is now available in the v3: https://github.com/ThomasAribart/json-schema-to-ts/issues/153 🙌

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

davidmurdoch commented 7 months ago

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. 🚀