bcherny/json-schema-to-typescript

Remove optional flag in case of `default` in schema.

kityan opened this issue · 8 comments

kityan commented

Correct me if I'm wrong, but nowadays the converter follows the strict rule: make prop optional if it's not in required list.
But JSON Schema allows using key default. Of course, existence of the prop in real object depends on realization (e.g.: schema-based validator).
I use ajv in my json-rpc server pipeline to validate arguments with useDefaults:true: it injects default values (defined in schema) in case of absent props.
That's why I'd like to have optional flag removed from such types.
If you feel this is good idea, I can try to fork and make a pull request soon. I think this behavior should be optional, e.g.: `removeOptionalIfDefaultExists"

kityan commented

May be it's not good idea at all. From a server side view it's OK to remove optional flag because default value is injected at validation stage.
But if we use the same type definition for a client — the optional flag should remain.

kityan commented

Anyway, the option I've suggested earlier will be useful. For example, in our development process we use two separate ts-files (with defaults injected - for server and without - for clients) converted from one json schema.
In this case client knows wich props are optional, and server thinks nothing optional because of injected defaults and we have less code in RPC methods.

Just add them to the required lists? In the past I had experience that required was checked before default so didn't work necessarily but now I'm setting required and having default and it works even when not given (i.e. defaulted passes required check). The issue with the proposal indeed is what if useDefaults:true is not given, required solves that.

"default" is an annotation keyword like "title" or "description". it doesn't make sense in the context of typescript types, as there is no such thing as "default value" for a type. On AJV side it becomes a footgun, since its value is not validated, so its type also can drift away from the schema type. Also some values can't be serialized or not known at validation time, such as home directory, so the type being optional or not varies on case-by-case basis on top.
Your mistake is using json schema as data modeling DSL instead of JSON payload validation DSL. JSON, and therefore JSON Schema, will never be able to map neatly into types of other languages.

@leppaott

Just add them to the required lists? In the past I had experience that required was checked before default so didn't work necessarily but now I'm setting required and having default and it works even when not given (i.e. defaulted passes required check). The issue with the proposal indeed is what if useDefaults:true is not given, required solves that.

Not really. Look — I use json schema files for:

  • validate request on backend
  • generate ts for backend
  • generate ts for frontend
  • generate docs (including defaults values!!!)

Also:

  1. I don't want to check at backend if optional param undefined and set to default in code. I want to do it by schema.
  2. I don't want to see at backend that the param is optional and use args.param! because I known — it is either sent by frontend or set to default by validator
  3. I want to see param as optional at frontend.

You see?
So my solution (when I generate ts for back and front) — to treat default: ... differently. For frontend I don't take it into account, only analyze "required:[...]" to see — optional or not. But for backend, I analyze default: .... to see if param is never undefined.

@GabenGar

Your mistake is using json schema as data modeling DSL instead of JSON payload validation DSL.

Don't judge before you try. So, your advice — don't use defaults in json schema, put this into code? Produce more imperative code instead of declarative? Funny.

@GabenGar

it doesn't make sense in the context of typescript types, as there is no such thing as "default value" for a type.

Of course. But it does make sense in context: would be param undefined or not (event if it's not required). So it's related to treating param is optional or not regardless it's not in required:[]