OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
22.02k stars 6.61k forks source link

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

Open Dunkhan opened 4 years ago

Dunkhan commented 4 years ago

Bug Report Checklist

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.

auto-labeler[bot] commented 4 years ago

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

agilob commented 4 years ago

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

jon-zu commented 4 years ago

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

runenielsen commented 3 years ago

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

agilob commented 3 years ago

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

runenielsen commented 3 years ago

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

darkbasic commented 3 years ago

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

BigApple1988 commented 3 years ago

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?

darkbasic commented 3 years ago

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

VincentVanclef commented 3 years ago

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

darkbasic commented 3 years ago

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

VincentVanclef commented 3 years ago

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

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!

mycarrysun commented 1 year ago

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

VincentVanclef commented 1 year ago

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

mycarrysun commented 1 year ago

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

t-rosa commented 1 year ago

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

mycarrysun commented 1 year ago

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

PetervdHemel commented 10 months ago

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?

guilhermedeborba commented 8 months ago

This also works image

koell-casa commented 7 months ago

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