openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][typescript-angular] All properties generated with "?:" even when they are not optional

Open Dunkhan opened this issue 4 years ago • 19 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [ ] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
Description

All properties generated by the openapi-generator are defined with ?: e.g.

/**
* Gets or sets the angle of reflection.
*/
angle?: number;

expected:

/**
* Gets or sets the angle of reflection.
*/
angle: number;
openapi-generator version

4.3.1

OpenAPI declaration file content or url

https://gist.github.com/Dunkhan/604694e5ae47c1162f784f1d3f3a78b9

Generation Details
npx openapi-generator generate -i D:\Work\testApi.json -g typescript-angular -c ./generate.api.options.json -o dist/api

generate options:

{  
   "npmName": "my-api",  
   "ngVersion": "9.1.0",
   "fileNaming": "kebab-case",
   "apiModulePrefix": "myPrefix"
}
Steps to reproduce

Run the command Look in the file \dist\api\model\effect-angle-values.ts

Related issues/PRs

none found

Suggest a fix

I am fully aware I might be doing something wrong, missing a flag, or reporting an intended behaviour. Please help me out if this is the case by explaining how I can get the generator to generate without the ?

I tried the flag nullSafeAdditionalProps but I found that this simply added "| null" to all nullable properties. This is good and I intend to keep using it but it does not solve the problem.

The reason this is an issue is that it I wish to use strict type checking and the properties being potentially undefined means that I have to fill all my code with undefined checks. These properties should never be undefined.

Dunkhan avatar Oct 01 '20 12:10 Dunkhan

👍 Thanks for opening this issue! 🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

auto-labeler[bot] avatar Oct 01 '20 12:10 auto-labeler[bot]

Your schema must be wrong. Required/optional fields are supported and tested for. In the example schema

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml

      parameters:
        - name: status
          in: query
          description: Status values that need to be considered for filter
          required: true
          type: array
          items:
            type: string
            enum:
              - available
              - pending
              - sold
            default: available

it generates correct output

image

add required: true

agilob avatar Oct 01 '20 17:10 agilob

@agilob I don't know how the tests look, but they are wrong. By definition nullable is false by default(https://swagger.io/specification/)! Yet not setting nullable at all treats nullable as true.

jon-zu avatar Oct 08 '20 15:10 jon-zu

@agilob I don't know how the tests look, but they are wrong. By definition nullable is false by default(https://swagger.io/specification/)! Yet not setting nullable at all treats nullable as true.

There's a difference between a field being nullable (controlled by the "nullable" property in the OpenAPI spec, which pr default is false) and a field being optional (controlled inversely by the "required" property in the OpenAPI spec, which pr default is also false).

Nullable is represented like this in TypeScript:

export interface Interface1 {
   nullableField: string | null;
}

Optional is represented like this in TypeScript:

export interface Interface2 {
   optionalField?: string;
}

A field can in theory be both nullable and optional, and the OpenAPI default is that fields are optional, but not nullable. I agree with agilob, that the behavior of the generator is correct.

The linked JSON https://gist.github.com/Dunkhan/604694e5ae47c1162f784f1d3f3a78b9 is missing "required: true" for the non-optional properties.

runenielsen avatar Apr 07 '21 20:04 runenielsen

And to add on top of your comment, a field can be required and nullable. Required means it needs to be specified with a value, and null is a value, as not specifying value will (optionally) use default value from spec. Gonna draw a Venn-diagram for this soon...

With that said, I think this issue can be closed now. @wing328

agilob avatar Apr 07 '21 20:04 agilob

@agilob: Agree. And to make it even more confusing - in TypeScript there's actually a third possibility, since undefined is also "kind of" a value: that the field IS present (so the object has the field key), but the value for the field key is set to undefined :-)

To be honest, I never really saw the value in differentiating between null and undefined in programming languages and tools. I've not yet in my career seen a case, where it made sense to differentiate, and it causes a lot of confusion (like @JonasZ95 comment above).

A funny side note is that the generated OpenAPI spec in a vanilla Spring Boot project in Java actually will be incorrect, since Jackson pr. default returns null values for missing properties, and OpenAPI pr. default uses required = false.

But if you always check with "!= null" og "== null" in TypeScript, you will never know the difference, since this works for both null and undefined.

runenielsen avatar Apr 08 '21 06:04 runenielsen

I have the same issue with typescript-axios: all properties are optional.

darkbasic avatar Jul 21 '21 13:07 darkbasic

For many of us "nullable" mostly equals to "optional", because it's really exotic scenario when we want to get parameter explicitly set to "null" value. On the other hand, even more exotic scenario is non-nullable optional properties. Why would you expect to get non-nullable property set?

Maybe there can be a generator option something like ''treatNonNullablePropertiesAsOptional"? which will generate non-nullable non-optional fields for non-nullable properties and nullable-and-optional fields for nullable properties?

BigApple1988 avatar Jul 30 '21 17:07 BigApple1988

I guess the root of the problem is badly made swagger definitions, I'm trying to get these fixed.

darkbasic avatar Jul 30 '21 21:07 darkbasic

@darkbasic Did you find a solution for this? I got the same issue with typescript-axios - everything is nullable.

VincentVanclef avatar Aug 18 '21 09:08 VincentVanclef

@VincentVanclef AFAIK you need to fix your swagger definitions, I didn't find any other way to force the properties to be non-optional

darkbasic avatar Aug 19 '21 08:08 darkbasic

@VincentVanclef AFAIK you need to fix your swagger definitions, I didn't find any other way to force the properties to be non-optional

I ended up adding this scema:

    {
        public void Apply(OpenApiSchema schema, SchemaFilterContext context)
        {
            if (schema.Properties == null)
            {
                return;
            }

            var notNullableProperties = schema
                .Properties
                .Where(x => !x.Value.Nullable && !schema.Required.Contains(x.Key))
                .ToList();

            foreach (var property in notNullableProperties)
            {
                schema.Required.Add(property.Key);
            }
        }
    }`


services.AddSwaggerGen(c =>
            {
                c.DescribeAllParametersInCamelCase();

                c.SupportNonNullableReferenceTypes();
                c.SchemaFilter<RequiredNotNullableSchemaFilter>();```


would this be how you did it as well?

VincentVanclef avatar Aug 19 '21 08:08 VincentVanclef

Hey guys, we are having the same issue, it's all pretty much optional in our models and we are using a bunch of "!" everywhere to convince the compiler that the value is indeed not undefined, but some resourses are always non-null non-optional!

fredowashere avatar Jun 05 '23 13:06 fredowashere

Your schema must be wrong. Required/optional fields are supported and tested for. In the example schema

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml

      parameters:
        - name: status
          in: query
          description: Status values that need to be considered for filter
          required: true
          type: array
          items:
            type: string
            enum:
              - available
              - pending
              - sold
            default: available

it generates correct output

image

add required: true

In your example it doesn't actually remove the status?: syntax that the OP was originally asking for. I'm looking for the behavior as well. Shouldn't have to use Required<MyGeneratedType> around everything. I'd like it to be smart enough to do status:.

mycarrysun avatar Nov 02 '23 18:11 mycarrysun

Your schema must be wrong. Required/optional fields are supported and tested for. In the example schema https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml

      parameters:
        - name: status
          in: query
          description: Status values that need to be considered for filter
          required: true
          type: array
          items:
            type: string
            enum:
              - available
              - pending
              - sold
            default: available

it generates correct output image add required: true

In your example it doesn't actually remove the status?: syntax that the OP was originally asking for. I'm looking for the behavior as well. Shouldn't have to use Required<MyGeneratedType> around everything. I'd like it to be smart enough to do status:.

Not sure what you mean, ive used this for years and it works perfectly - respects nullabilty/optional etc

VincentVanclef avatar Nov 02 '23 19:11 VincentVanclef

In your screenshot it generated status?: Pet.StatusEnum. We want status: Pet.StatusEnum.

mycarrysun avatar Nov 02 '23 20:11 mycarrysun

Have you tried using the [Required] annotation at property level? It worked for me, but I use openapi-typescript-codegen. image

t-rosa avatar Nov 23 '23 21:11 t-rosa

Have you tried using the [Required] annotation at property level? It worked for me, but I use openapi-typescript-codegen. image

We didn't want to have to do that on literally every property throughout the code base

mycarrysun avatar Nov 25 '23 23:11 mycarrysun

Hi I wanted to add to this, and at the same time ask for help as I'm also running into issues, having to constantly assure Typescript my API isn't returning objects with properties that can be of type undefined...

I am using FastAPI's OpenAPI schema generator to create an openapi.json file, which I then run through the typescript-axios tool (v7.2.0) for use in my front-end application.

What I've found is that, even though I define properties in FastAPI as having default values like so:

class Example(BaseModel):
    name: str = Field(default="Example Name")

Which results in the following schema in the openapi.json file:

   "Example": {
    "properties": {
     "name": {
      "type": "string",
      "title": "Name",
      "default": "Example Name"
     },

But then the resulting generated interface states:

export interface Example {
    /**
     *
     * @type {string}
     * @memberof Example
     */
    'name'?: string;

It simply doesn't make sense (to me) why 'name' should be optional here as it is seen by Typescript to be of type Example['name']?: string | undefined which causes me to have to constantly tell TS it is not undefined.

Schemas having default values means they will never be undefined in this case, so why generate them as optional?

PetervdHemel avatar Jan 19 '24 11:01 PetervdHemel

This also works image

guilhermedeborba avatar Mar 11 '24 23:03 guilhermedeborba

Running in the same issue here. Required properties shouldn't be generated with "?"

koell-casa avatar Apr 18 '24 07:04 koell-casa