Brakebein / prisma-generator-nestjs-dto

Generates NestJS DTO classes from Prisma Schema
Apache License 2.0
51 stars 28 forks source link

Missing type in @ApiProperty for optional strings #38

Closed mirrom closed 7 months ago

mirrom commented 8 months ago

A simple model in the schema.prisma:

model SimpleModel {
  requiredString String
  optionalString String?
}

This generates a simple-model.dto.ts:

export class SimpleModel {
  @ApiProperty()
  requiredString: string;
  @ApiProperty({
    nullable: true,
  })
  optionalString: string | null;
}

When using @nestjs/swagger, the OpenAPI definition for the SimpleModelDto looks like this:

{
  "openapi": "3.0.0",
  "components": {
    "schemas": {
      "SimpleModelDto": {
        "type": "object",
        "properties": {
          "requiredString": {
            "type": "string"
          },
          "optionalString": {
            "type": "object",
            "nullable": true
          }
        }
      }
    }
  }
}

As you can see, the type of the optionalString is "object" instead of "string". This leads to problems when using a generator in the frontend like e.g. OpenAPI Generator.

When including the type in the @ApiProperty like:

export class SimpleModel {
  @ApiProperty({
    type: 'string',
    nullable: true,
  })
  optionalString: string | null;
}

the resulting OpenAPI json sets the correct type:

{
  ...
          "optionalString": {
            "type": "string",
            "nullable": true
          }
  ...
}

I did not find any option to force the generator to include the type in the @ApiProperty. I could extend the generated dtos and manually set the types for all optional strings, but this somehow contradicts the purpose of generating classes. So I would love to see either adding types for optional strings or having a global option to always include the types in the @ApiProperty (and all other relevant decorators, I guess).

Brakebein commented 8 months ago

Which version of @nestjs/swagger are you using?

npm ls @nestjs/swagger
mirrom commented 8 months ago

Sorry, you are right, I missed adding version information!

@nestjs/core: 10.3.4 @nestjs/swagger: 7.3.0 @brakebein/prisma-generator-nestjs-dto: 1.20.0 prisma: 5.11.0

Actually I wonder if this isn't a bug of the @nestjs/swagger package as it sets the correct type on mandatory strings even if you do not specify it via @ApiProperty, but sets it as "object" if it is an optional string. Anyway, having the generator explicitly set the type would be a nice workaround ;)

Brakebein commented 8 months ago

I have two projects running: 1. is using @nestjs/swagger@7.1.4 where the same bug appears as described above. 2. is using @nestjs/swagger@7.1.15 where everything looks fine. I may need to look deeper into it.

Brakebein commented 7 months ago

Hm, I updated the one project to @nestjs/swagger@7.1.15 as well, but it's still not inferring the type on optional fields. The DTOs look pretty much the same. I don't know what's the difference between the one project where it just works fine, and the other with same issue as described by you. I think the easiest solution is to always explicitly add the type property as you suggested.

Brakebein commented 7 months ago

Please check out version 1.21.0-beta, which now explicitly sets the type property of the @ApiProperty decorator.

mirrom commented 7 months ago

Beautiful, exactly what I needed! Works perfectly fine for me, I could basically remove all dtos that extended the generated dtos.

Like I said, I guess the actual bug is not in this package but somewhere in the @nestjs/swagger package. Your changes though make the generated dtos very explicit which I think is a pretty good thing :)

Thanks a lot for your support!