Brakebein / prisma-generator-nestjs-dto

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

Circular reference breaks everything #49

Open Serpentarius13 opened 2 months ago

Serpentarius13 commented 2 months ago

I have multiple tables with one-to-one and one-to-many relationships, for example:

model Delivery {
  id Int @id @default(autoincrement())

  items DeliveryItem[]

  delivered_at           DateTime
  estimated_delivered_at DateTime?

  status DeliveryStatus

  car_name String

  supplier_id Int
  supplier    Supplier @relation(fields: [supplier_id], references: [supplier_id])

  client_id Int
  client    Client @relation(fields: [client_id], references: [client_id])

  warehouse_id Int
  warehouse    Warehouse @relation(fields: [warehouse_id], references: [warehouse_id])

  created_at DateTime @default(now())
  updated_at DateTime @updatedAt

  @@map("delivery")
}

enum DeliveryItemType {
  PALETTE
  BOX
  ITEMS
}

model DeliveryItem {
  id Int @id @default(autoincrement())

  delivery_id Int
  delivery    Delivery @relation(fields: [delivery_id], references: [id])

  good_id Int
  good    Good @relation(fields: [good_id], references: [id])

  type DeliveryItemType

  approx_per_item Int?
  per_item        Int?

  @@map("delivery_item")
}

And it generates code like this:

import { ApiProperty } from "@nestjs/swagger";
import { DeliveryItemType } from "@prisma/client";
import { Delivery } from "./delivery.entity";
import { Good } from "./good.entity";

export class DeliveryItem {
  @ApiProperty({
    type: "integer",
    format: "int32",
  })
  id!: number;
  @ApiProperty({
    type: "integer",
    format: "int32",
  })
  delivery_id!: number;
  @ApiProperty({
    type: () => Delivery,
    required: false,
  })
  delivery?: Delivery;
  @ApiProperty({
    type: "integer",
    format: "int32",
  })
  good_id!: number;
  @ApiProperty({
    type: () => Good,
    required: false,
  })
  good?: Good;
  @ApiProperty({
    enum: DeliveryItemType,
  })
  type!: DeliveryItemType;
  @ApiProperty({
    type: "integer",
    format: "int32",
    nullable: true,
  })
  approx_per_item!: number | null;
  @ApiProperty({
    type: "integer",
    format: "int32",
    nullable: true,
  })
  per_item!: number | null;
}

when generating @ApiProperty types is enabled (I need it for swagger to show fields correctly in UI). However, I'm getting errors like that one: image

I think it is related to circular references when using class as a type for property in generated entities. This approach worked for me, but I don't know how to extend generator on my own :( Please help

Serpentarius13 commented 2 months ago

Actually, the issue shows up here also: https://github.com/Brakebein/prisma-generator-nestjs-dto/issues/39

Brakebein commented 2 months ago

Yes, this is a bit of an annoying issue with SWC.

Can you try if the newly introduced config outputApiPropertyType = "false" works in combination with the swagger cli-plugin as described below?

What I found out is, that it's okay to omit the type property in @ApiProperty IF you have the swagger cli-plugin setup (https://docs.nestjs.com/openapi/cli-plugin). This plugin will "set the type or enum property depending on the type (supports arrays as well)" apparently in a manner where SWC has no issue with it and swagger can still derive the actual schema from it.

Meaning that if we add an option that omits the type property in @ApiProperty and lean on the cli-plugin it works with swagger and with SWC without having to change much.

https://github.com/Brakebein/prisma-generator-nestjs-dto/issues/39#issuecomment-2168651849

Serpentarius13 commented 2 months ago

Yes, this is a bit of an annoying issue with SWC.

Can you try if the newly introduced config outputApiPropertyType = "false" works in combination with the swagger cli-plugin as described below?

What I found out is, that it's okay to omit the type property in @ApiProperty IF you have the swagger cli-plugin setup (https://docs.nestjs.com/openapi/cli-plugin). This plugin will "set the type or enum property depending on the type (supports arrays as well)" apparently in a manner where SWC has no issue with it and swagger can still derive the actual schema from it. Meaning that if we add an option that omits the type property in @ApiProperty and lean on the cli-plugin it works with swagger and with SWC without having to change much.

#39 (comment)

I will try, thank you

Serpentarius13 commented 2 months ago

@Brakebein Hello again. I've tried using swagger plugin and I think it's not the best solution to fix this problem. It doesn't even really work, here's an explanation. I have nullable Prisma fields which are generated like so with outputApiPropertyType = "false":

  @ApiProperty({
    nullable: true,
  })
  surname!: string | null;

but in my Swagger schema they popup like this, even when using plugin, which I think is unusable, because there is no way to figure out which type is the field from looking at it: image

However, when writing property like this:

  @ApiProperty({
    nullable: true,
    type: String,
  })
  example!: string | null;

I'm getting the correct type even without example: image So, when using this plugin, I'm forced to annotate nullable fields in prisma.schema with /// @example, otherwise they all are empty objects. Can we make the fix happen like its mentioned in the issue I linked? It doesn't seem hard to me, would it be costly?

Brakebein commented 2 months ago

I wrapped the imports as types as in the referenced issue. Please test out 1.24.0-beta0 and check if it solves the issue. For that, I introduced a new config: wrapRelationsAsType = "true"

Serpentarius13 commented 2 months ago

I wrapped the imports as types as in the referenced issue. Please test out 1.24.0-beta0 and check if it solves the issue. For that, I introduced a new config: wrapRelationsAsType = "true"

Cool! Will check it out

Brakebein commented 4 weeks ago

Can you please give a short feedback, if it now works as expected?

Serpentarius13 commented 4 weeks ago

Can you please give a short feedback, if it now works as expected?

Sorry, I have migrated away from the generator as I figured it would take some time to push new features I needed if they ever would be there as it is a lot of job to do so. Thank you for your work!

Brakebein commented 4 weeks ago

Okay, that's fine.