DtoCastType is being ignored

[stefangeyer]

Hello! Could it be that @DtoCastType is currently broken? During my tests, the following schema snippet will result in the cast being ignored. It seems like the cast import is generated correctly, but the type annotation is not updated.

Due to internal reasons, I need to express my m2m explicitly - they are transformed to the implicit counterpart for the API response, however. Therefore, I need a way to adjust the OpenAPI response model for the generated docs, which is why I would like to cast the m2m relation fields. I am thankful for any help on this matter!

generator model {
  provider            = "prisma-generator-nestjs-dto"
  output              = "../src/generated/model"
  fileNamingStyle     = "kebab"
  classValidation     = "true"
  prettier            = "true"
}

model Team {
  id            Int                @id @default(autoincrement())
  createdAt     DateTime           @default(now())
  updatedAt     DateTime           @updatedAt
  name          String
  about         String?
  thumbnailUrl  String?
  photoUrl      String?
  externalId    String?
  externalHost  ExternalHostType?
  /// @DtoApiHidden
  registrations TeamRegistration[]
  /// @DtoCastType(Player, ./player.entity)
  members       PlayerOnTeam[]

  @@unique([externalId, externalHost])
}

import { ExternalHostType } from "@prisma/client";
import { Player } from "./player.entity";
import { ApiHideProperty, ApiProperty } from "@nestjs/swagger";
import { TeamRegistration } from "./team-registration.entity";
import { PlayerOnTeam } from "./player-on-team.entity";

export class Team {
  @ApiProperty({
    type: "integer",
    format: "int32",
  })
  id: number;
  @ApiProperty({
    type: "string",
    format: "date-time",
  })
  createdAt: Date;
  @ApiProperty({
    type: "string",
    format: "date-time",
  })
  updatedAt: Date;
  @ApiProperty({
    type: "string",
  })
  name: string;
  @ApiProperty({
    type: "string",
    nullable: true,
  })
  about: string | null;
  @ApiProperty({
    type: "string",
    nullable: true,
  })
  thumbnailUrl: string | null;
  @ApiProperty({
    type: "string",
    nullable: true,
  })
  photoUrl: string | null;
  @ApiProperty({
    type: "string",
    nullable: true,
  })
  externalId: string | null;
  @ApiProperty({
    enum: ExternalHostType,
    nullable: true,
  })
  externalHost: ExternalHostType | null;
  @ApiHideProperty()
  registrations?: TeamRegistration[];
  @ApiProperty({
    type: () => PlayerOnTeam,
    isArray: true,
    required: false,
  })
  members?: PlayerOnTeam[];
}

[Brakebein]

Currently, the DtoCastType annotation only works on scalar fields and not on relation fields. And ApiProperty type is not affected, but will be resolved from the original field type. So this feature is quite basic at the moment.

Do you need it to be cast only for the class property type, only for the ApiProperty type, or for both?

[stefangeyer]

Assuming a service flattens the data and returns an instance satisfying the entity type, casting the property type and the ApiProperty type is the way to go in this case (I mainly need all entities to be present in the DMMF which is not the case with relation tables of many-to-many relationships in prisma by default). Only casting the ApiProperty type would mean that a service returns an instance of the explicit many-to-many relation, while the API returns the implicit version. Further, this would mean that the conversion needs to happen within a controller which is most definitely the wrong place for it 🤔.

All other property types are consistent with the type of their ApiProperty type, correct? It would be unexpected behavior for the types to differ if a cast is used.

[Brakebein]

All other property types are consistent with the type of their ApiProperty type, correct? It would be unexpected behavior for the types to differ if a cast is used.

I think, the reason that the DtoCastType annotation only applies to the property type is that it was designed to cast the type to something from an external package. Whereas for ApiProperty type, it would have made no sense, since the imported type or class doesn't have any swagger decorators or anything alike, or would even break it. In the original PR (#12) we were only talking about Json fields that would be properly typed after casting.

So, I'm not sure, if changing the behavior such that also ApiProperty type is casted, if it would break anything. Or if it needs some special annotation that has this different behavior (e.g. DtoCastTypeFull or something with a better name).

[stefangeyer]

I think, the reason that the DtoCastType annotation only applies to the property type is that it was designed to cast the type to something from an external package.

Is there a reason why this only applies to scalars? I don't see many ways this could lead to unexpected errors as the annotation needs to be provided explicitly for the cast to happen. If the user wants to override the type of a relation to something else explicitly, why not let them?

So, I'm not sure, if changing the behavior such that also ApiProperty type is casted, if it would break anything. Or if it needs some special annotation that has this different behavior (e.g. DtoCastTypeFull or something with a better name).

I think it would be a good idea to split up the overrides for the property type and the ApiProperty type into two separate annotations. This would provide maximum flexibility depending on the use case. Could be named DtoOverrideType and DtoApiOverrideType.

[Brakebein]

Is there a reason why this only applies to scalars? I don't see many ways this could lead to unexpected errors as the annotation needs to be provided explicitly for the cast to happen. If the user wants to override the type of a relation to something else explicitly, why not let them?

Yes, overriding the property type shouldn't break anything. As I said, the only reason, for which the current behavior is as it is now, is that the original PR was only focusing on Json fields. So overriding the relation fields should be no problem in general. My only concern above was the (automatic) override of the ApiProperty types.

I think it would be a good idea to split up the overrides for the property type and the ApiProperty type into two separate annotations. This would provide maximum flexibility depending on the use case. Could be named DtoOverrideType and DtoApiOverrideType.

These are better (or more precise) namings of the annotations. Also better than DtoCastType, in my opinion. That's something I can implement. Hence, DtoOverrideType would replace DtoCaseType (which would be deprecated), and DtoApiOverrideType (same syntax) would be added.

[stefangeyer]

Would be highly appreciated as I currently don't have the resources to provide a PR. Thank you very much.

[Brakebein]

You can now please test 1.24.0-beta2.

It should now work for all fields including relation fields with both new annotations as stated above.

However, there is some not very clear behavior when it comes to relation fields with annotions like @DtoRelationCanCreateOnCreate or @DtoRelationCanConnectOnUpdate, etc. Then, the CreateDtos and UpdateDtos are getting a bit more complex. Now, @DtoOverrideType will also replace the linked Create/UpdateDtos. This also applies similarly to complex types (mongodb). So, DtoOverrideType and DtoApiOverrideType should only be used with great care.

[stefangeyer]

Thanks a bunch! I will hopefully get around to testing within the next week.

[Brakebein]

Did you find some time to test it?

[stefangeyer]

Hello! Sorry for the delay - I am finally getting to test this. Here are my first findings:

The following model is used for the provided test cases:

model Team {
  id            Int                @id @default(autoincrement())
  createdAt     DateTime           @default(now())
  updatedAt     DateTime           @updatedAt
  name          String
  about         String?
  thumbnailUrl  String?
  photoUrl      String?
  externalId    String?
  externalHost  ExternalHostType?
  /// @DtoApiHidden
  registrations TeamRegistration[]
  members       PlayerOnTeam[]

  @@unique([externalId, externalHost])
}

Test case 1 - using @DtoOverrideType isolated:

/// @DtoOverrideType(Player, ./player.entity)
members       PlayerOnTeam[]

Produces correct result.

@ApiProperty({
  type: () => PlayerOnTeam,
  isArray: true,
  required: false,
})
members?: Player[];

Test case 2 - using @DtoApiOverrideType isolated:

/// @DtoApiOverrideType(Player, ./player.entity)
members       PlayerOnTeam[]

Produces correct result.

@ApiProperty({
  type: Player,
  isArray: true,
  required: false,
})
members?: PlayerOnTeam[];

Test case 3 - using @DtoOverrideType and @DtoOverrideApiType in combination:

/// @DtoOverrideType(Player, ./player.entity)
/// @DtoApiOverrideType(Player, ./player.entity)
members       PlayerOnTeam[]

Produces correct result.

@ApiProperty({
  type: Player,
  isArray: true,
  required: false,
})
members?: Player[];

Summary:

@DtoOverrideType - seems to work as expected @DtoApiOverrideType - seems to work as expected

[Brakebein]

Thanks! However, it's not @DtoOverrideApiType, but @DtoApiOverrideType as you have proposed in one of your comments above. But maybe @DtoOverrideApiType is more intuitive?

[stefangeyer]

Ah, my bad... One shouldn't do stuff like this in a rush 🙃 - I will update my comment. Regarding the name: I would keep it consistent with whatever other annotations you have concerning @ApiProperty

Maybe @DtoOverrideApiPropertyType would make the most sense even...

[stefangeyer]

Also on an unrelated note:

  /// @DtoOverrideType(Player, ./player.entity)
  /// @DtoApiOverrideType(Player, ./player.entity)
  members       PlayerOnTeam[]

will generate

  @ApiProperty({
    type: Player,
    isArray: true,
    required: false,
  })
  members?: Player[];

which will then again produce

  "members": {
    "type": "array",
    "items": {
      "required": false,
      "type": "array",
      "items": {
        "$ref": "#/components/schemas/Player"
      }
    }
  }

in comparison to if I add no annotations at all

  @ApiProperty({
    type: () => PlayerOnTeam,
    isArray: true,
    required: false,
  })
  members?: PlayerOnTeam[];

which produces

  "members": {
    "type": "array",
    "items": {
      "$ref": "#/components/schemas/PlayerOnTeam"
    }
  }

It would seem like overriding the type from a function to providing the class explicitly will mess up the typing, exposing the data type as a two-dimensional array, which is incorrect.

Manually changing the @ApiProperty type to

  @ApiProperty({
    type: () => Player,
    isArray: true,
    required: false,
  })
  members?: Player[];

produces the correct result:

  "members": {
    "type": "array",
    "items": {
      "$ref": "#/components/schemas/Player"
    }
  }

[Brakebein]

I changed the annotation to @DtoOverrideApiPropertyType. It's more explicit and better distinguishable from the other one. The type property is now provided as funktion. 1.24.0-beta5

[stefangeyer]

Awesome - the overrides seem to work now!

I have one more question regarding the use of the @DtoRelationRequired annotation:

  /// @DtoRelationRequired
  /// @DtoOverrideApiPropertyType(Player, ./player.entity)
  members       PlayerOnTeam[]

generates

  @ApiProperty({
    type: () => Player,
    isArray: true,
    required: false,
  })
  members?: PlayerOnTeam[];

Using @DtoRelationRequired does not seem to force the required field. Am I missing something?

[Brakebein]

I actually haven't touched the @DtoRelationRequired annotation and its behavior since I've forked the repo. Nor did I use it in my projects. The readme states that it should be required as you would assume by the name. However, the example also shows optional properties. I will have a look into it.

[KoenLemmen]

Did you perchance have the time to look at @DtoRelationRequired. I noticed that for my interface exports for the client side relations are also given the TypeScript ?, which I did not expect.