Manweill / swagger-axios-codegen

swagger client to use axios and typescript
MIT License
306 stars 83 forks source link

Support for nullable? #31

Closed pumpkinlink closed 2 years ago

pumpkinlink commented 5 years ago

Does this codegen support translating swagger nullable data type attribute as typescript undefined/null/question mark?

Manweill commented 5 years ago

For example ?

Manweill commented 5 years ago

@pumpkinlink
You can set strictNullChecks = false

pumpkinlink commented 5 years ago

Sorry for being so vague, I'll try to explain

I was thinking about something like in java's typescript-generator optionalProperties and optionalPropertiesDeclaration options where I can, for instance, generate question-marked typescript properties based on if the JsonProperty(required) annotation is set to true or false on each specific java attribute.

The point is to make some json properties required on a POST request without having to rely only on runtime server-side validation.

My team is analyzing about migrating our existing stack from Java/Spring→Typescript into Elixir/typespec→Swagger spec→Typescript. Our current setup generates both the domain type definitions and the axios services, similarly to what swagger-axios-codegen does, but the translation is direct, without Swagger involved.

I checked Swagger docs for something similar and found the referred link about the "nullable" data-type, which is exactly what I was looking for. So I was asking if swagger-axios-codegen takes this in account or not. strictNullChecks = false seems good enough for now though, but would you accept a PR with a per-attribute support based on swagger's "nullable"?

Below is an example of one the only DTO where we use this feature, java on left, typescript on right: image

Manweill commented 5 years ago

@pumpkinlink Yes, I will accept if it is reasonable. Welcome~

joshstrange commented 4 years ago

@Manweill Thank you for this library! I was wondering how you think would be best to address this. Not sure if you want the IPropDef updated to have another property of nullable: boolean or we can re-used the validationModel property. Doing a little surgical editing I took this line:

    ${props.map(p => classPropsTemplate(p.name, p.type, p.format, p.desc, !strictNullChecks, false, false)).join('')}

to

    ${props.map(p => classPropsTemplate(p.name, p.type, p.format, p.desc, !strictNullChecks || !p.validationModel || !p.validationModel.required, false, false)).join('')}

The difference being the addition of !p.validationModel || !p.validationModel.required along with !strictNullChecks.

Since in my testing I was seeing p.validationModel be either null or validationModel: { required: true }. Now that's just with my schema so I'm not sure what else might be in there for other people.

This change gave me the behavior I was looking for (nullable properties now have the '?' on them). I'm happy to put together a pull request for just that change to the interface/class code. Let me if that is acceptable or if you were looking for something bigger/different.

All that said my gut feeling is even this isn't the best approach or should be behind a flag (nullableBehavior: 'optional' | 'null-type' | 'both'). I think I would prefer to have the type written out like myProp: string | null (assuming myProp was a string in Swagger). But I can understand some people might want the ? added as well so they don't even have to define it as null to pass type checks. Let me know your thoughts.

Manweill commented 4 years ago

like this: myProp:string|null|undefined

LuccaPrado commented 3 years ago

Any update to that?

pumpkinlink commented 3 years ago

Any update to that?

In my case I'm unfortunately no longer working on the project I mentioned, so don't expect a PR from me anymore, at least on the foreseeable future... :/