acacode / swagger-typescript-api

Generate the API Client for Fetch or Axios from an OpenAPI Specification
MIT License
3.35k stars 361 forks source link

Non nullable properties are generated as possibly undefined #396

Open jekcom opened 2 years ago

jekcom commented 2 years ago

I have an object of a product that has some non nullable properties such as ProductId and price with the following schema generated by swagger

"ProductBasicInfo": {
        "type": "object",
        "properties": {
          "productId": {
            "type": "integer",
            "format": "int32"
          },
          "brand": {
            "type": "string",
            "nullable": true
          },
          "description": {
            "type": "string",
            "nullable": true
          },
          "name": {
            "type": "string",
            "nullable": true
          },
          "price": {
            "type": "number",
            "format": "double"
          }
        },
        "additionalProperties": false
      }

This is the generated contract

export interface ProductBasicInfo {
  /** @format int32 */
  productId?: number;
  brand?: string | null;
  description?: string | null;
  name?: string | null;

  /** @format double */
  price?: number;
}

As you can see, in the generated contract a ? was added to the non nullable properties making their type effectively number | undefined

Is it possible to omit the ? mark for non nullable types?

rkuykendall commented 2 years ago

I am having the same issue.

rkuykendall commented 2 years ago

We are using swagger-typescript-api 9.2.0 and swagger 2.1.13. @jekcom what versions are you on?

jekcom commented 2 years ago

We are using swagger-typescript-api 9.2.0 and swagger 2.1.13. @jekcom what versions are you on?

swagger-typescript-api 9.2.0 "openapi": "3.0.1"

rkuykendall commented 2 years ago

We are also on openapi 3.0.1. I checked, and the issue is happening in the latest swagger-typescript-api as well.

remkoboschker commented 2 years ago

I think the issues is that the open-api spec states that nullable true is a way to say that a property may have the value null. To express that a property may not be undefined would require listing the property as required for that object.

HansFalkenberg-Visma commented 2 years ago

What @remkoboschker said is correct. The issue here seems to be the API specification, not this generator.

Unless the name of an OpenApi property is included in the object's required list, it is an optional property. Optional properties are expressed with ? or undefined in TypeScript.

Nullability is something completely different, namely whether the value of a property that is present may be null. Expressed as null in TypeScript.

I would also recommend simply never using null in TypeScript. The scenarios where it is truly needed are few and far between and the language support for undefined is much better. This works out if any C#/Java implementations -- which don't have a native concept of undefined -- treat missing properties as null. If you use both null and undefined in TypeScript and then expect those to be different things (which they totally are!), it likely won't work out when you meet a C#/Java implementation.

jekcom commented 2 years ago

@HansFalkenberg-Visma in c# for example, a value types can never be null (nor optional) unless you specify explicitly that they are nullable, and a case scenario for this is rather rare. Thus marking explicitly each and every value type property as required sounds a bit redundant and unnecessarily "boiler platy" . So when converting c# type into typescript type i would expect it to follow the original language constraints and not mark a non nullable, non optional property as optional because by they nature they are not and cannot be such

HansFalkenberg-Visma commented 2 years ago

@jekcom That is true, but there seems to be a misunderstanding on another level then.

This project generates TypeScript code from an OpenApi/swagger schema definition, written in YAML or JSON. C# code is not that. swagger-typescript-api cannot convert C# into TypeScript.

So the incorrect constraints put into the JSON schema in your original post needs to be addressed with whatever created it from your C# code.

KRSogaard commented 10 months ago

Just to update to this, here is an example of the generated code from C#, you will see that some fields have "nullable": true where others do not. This is within the OpenAPI specification and is valid.

https://swagger.io/docs/specification/data-models/data-types/

OpenAPI 3.0 does not have an explicit null type as in JSON Schema, but you can use nullable: true to specify that the value may be null. Note that null is different from an empty string "".

"HydratedLeaseDto": {
        "title": "HydratedLease",
        "type": "object",
        "properties": {
          "id": {
            "type": "string",
            "nullable": true
          },
          "unitId": {
            "type": "string",
            "nullable": true
          },
          "propertyId": {
            "type": "string",
            "nullable": true
          },
          "portfolioId": {
            "type": "string",
            "nullable": true
          },
          "start": {
            "type": "string",
            "format": "date-time"
          },
          "end": {
            "type": "string",
            "format": "date-time",
            "nullable": true
          },
          "monthToMonth": {
            "type": "boolean"
          },
          "eviction": {
            "type": "boolean"
          },
          "chatId": {
            "type": "string",
            "nullable": true
          },
          "tenants": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/HydratedLeaseTenantDto"
            },
            "nullable": true
          },
          "balance": {
            "type": "number",
            "format": "double"
          },
          "currentRent": {
            "type": "number",
            "format": "double"
          },
          "currentDeposit": {
            "type": "number",
            "format": "double"
          },
          "lastPayment": {
            "type": "string",
            "format": "date-time",
            "nullable": true
          },
          "earliestUnpaidBillDue": {
            "type": "string",
            "format": "date-time",
            "nullable": true
          },
          "unit": {
            "$ref": "#/components/schemas/UnitDto"
          },
          "property": {
            "$ref": "#/components/schemas/PropertyDto"
          }
        },
        "additionalProperties": false
      },
wengerari commented 10 months ago

How can i tern off undefined type by contract generating. It'is the best way to handle IT, isn't IT?

noah-rss commented 9 months ago

@wengerari and others -

In my company's case, there is no way that our request / response models need to handle the 'undefined' case. Every property should be specified in either direction, regardless of nullability.

Because of this, we modified the call to generateApi to remove the generation of undefined properties. If anyone else is in the same situation, here's how you can do that:

generateApi({
  codeGenConstructs: constructs => ({
    ...constructs,
    TypeField: ({ readonly, key, value }) =>
      [...(readonly ? ['readonly '] : []), key, ': ', value].join(''),
  })
})

Hope this helps!

Matster2 commented 7 months ago

it seems though that it is nullable by default. My spec is generated from an asp.net core api. On the response type, these properties do not allow for null. When the spec is generated, it makes no reference to "nullable": true image

But when i generate my api client using generateApi, they are turned into optional variables image

im a little confused

edproton commented 7 months ago

it seems though that it is nullable by default. My spec is generated from an asp.net core api. On the response type, these properties do not allow for null. When the spec is generated, it makes no reference to "nullable": true image

But when i generate my api client using generateApi, they are turned into optional variables image

im a little confused

Im having the same problem

this is my schema

image

this is what is generated

image

Maskoe commented 6 months ago

If you're here from AspNetCore and its automatic swagger schema generation. You can make the swagger schema interpret the c# required keyword like you want with this code.

public class RequiredSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        var requiredProps = context.Type.GetProperties()
            .Where(x => x.GetCustomAttribute<RequiredMemberAttribute>() is not null)
            .ToList();

        var requiredJsonProps = schema.Properties
            .Where(j => requiredProps.Any(p => p.Name == ToPascalCase(j.Key)))
            .ToList();

        schema.Required = requiredJsonProps.Select(x => x.Key).ToHashSet();

        // Optionally set them non nullable too.
        // foreach (var requiredJsonProp in requiredJsonProps)
        //     requiredJsonProp.Value.Nullable = false;
    }

    public string ToPascalCase(string str)
    {
        if (string.IsNullOrEmpty(str))
            return str;

        return char.ToUpper(str[0]) + str.Substring(1);
    }
}

// Program.cs
builder.Services.AddSwaggerGen(x =>  x.SchemaFilter<RequiredSchemaFilter>());

// Sample class
public record WeatherRequest
{
    public required string City { get; set; }
    public required string Country { get; set; }
    public string Name { get; set; }
}

This will take the sample WeatherRequest and create this schema

"WeatherRequest": {
  "required": [
    "city",
    "country"
  ],
  "type": "object",
  "properties": {
    "city": {
      "type": "string",
      "nullable": true
    },
    "country": {
      "type": "string",
      "nullable": true
    },
    "name": {
      "type": "string",
      "nullable": true
    }
  },
  "additionalProperties": false
}

This will let this generator generate the correct client.

AspNetCore won't allow you to omit those properties on a request. It will just return a 400 BadRequest.

Oddly its still possible to directly post null values with this. I've not found a way of handling that with JsonSerialization or manipulating the generated swagger schema.

I just added a middlleware that checks whether any properties on the incoming type are marked as required and if they have null values, I return a BadRequest manually. Since I'm a FastEndpoints enjoyer here is the code, I'm sure you can adjust it to standard middleware too.

public async Task PreProcessAsync(IPreProcessorContext ctx, CancellationToken ct)
    {
        if (ctx.Request is null)
            return;

        var unallowedNullFailures = ctx.Request.GetType().GetProperties()
            .Where(x => x.GetCustomAttribute<RequiredMemberAttribute>() is not null)
            .Where(x => x.GetValue(ctx.Request) is null)
            .Select(x => new ValidationFailure(x.Name, $"{x.Name} is a required property. It can't be deserialzed to null."))
            .ToList();

        if (unallowedNullFailures.Any())
            await ctx.HttpContext.Response.SendErrorsAsync(unallowedNullFailures);
    }

And this is how you manipulate the schema in NSwag / FastEndpoints

public class RequiredSchemaProcessor : ISchemaProcessor
{
    public void Process(SchemaProcessorContext context)
    {
        var requiredProps = context.ContextualType.Type.GetProperties()
            .Where(x => x.GetCustomAttribute<RequiredMemberAttribute>() is not null)
            .ToList();

        var requiredJsonProps = context.Schema.Properties
            .Where(j => requiredProps.Any(p => p.Name == j.Key.ToPascalCase()))
            .ToList();

        foreach (var requiredJsonProp in requiredJsonProps)
            requiredJsonProp.Value.IsRequired = true;
    }
}

I have no idea why either of these are not built in. Feels really strange. The reason I stayed away from nullable reference types until now, is because they were just kinda lies. You could easily get null values from a client. But this way you can't and you can actually treat a non nullable string as really for real not null.