Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Include typings when having models called X and XUpdate #7518

Closed
Tracked by #7963
ianmartorell opened this issue Jun 8, 2021 · 8 comments · Fixed by #20184
Closed
Tracked by #7963

Incorrect Include typings when having models called X and XUpdate #7518

ianmartorell opened this issue Jun 8, 2021 · 8 comments · Fixed by #20184
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. tech/typescript Issue for tech TypeScript. topic: client types Types in Prisma Client topic: reserved words See https://www.prisma.io/docs/reference/api-reference/prisma-schema-reference#naming-conventions topic: type-clash
Milestone

Comments

@ianmartorell
Copy link

Bug description

When having two models called X and XUpdate, the generated types for include parameters of XUpdate are incorrect.

How to reproduce

Example (only relevant fields included):

model Product {
  id      String          @id @default(uuid())
  updates ProductUpdate[] @relation(name: "ProductUpdates")
}

model ProductUpdate {
  id        String  @id @default(uuid())
  productId String
  product   Product @relation(name: "ProductUpdates", fields: [productId], references: [id])
}

Then, when querying another model that has a relation with ProductUpdate:

prisma.orderItem.findUnique({
  where: {
    id,
  },
  include: {
    productUpdate: {
      include: {
        product: true,
      },
    },
  },
})

I get the following error:

Screenshot 2021-06-08 at 10 15 54

Looking at the generated types, this seems to be because the productUpdate field in OrderItemInclude has type ProductUpdateArgs, which are the arguments for an update operation on a Product model, instead of the args without action of the ProductUpdate model.

Screenshot 2021-06-08 at 10 16 41

Screenshot 2021-06-08 at 10 18 47

Expected behavior

The type of OrderItemInclude.productUpdate should be related to the ProductUpdate model, not the Product model. Consequently there should not be a type error in this situation.

Prisma information

Relevant prisma schema provide above.

Environment & setup

  • OS: macOS
  • Database: PostgreSQL
  • Node.js version: 15.14.0

Prisma Version

Environment variables loaded from .env
@prisma/cli          : 2.13.1
@prisma/client       : 2.13.1
Current platform     : darwin
Query Engine         : query-engine fcbc4bb2d306c86c28014f596b1e8c7980af8bd4 (at node_modules/@prisma/engines/query-engine-darwin)
Migration Engine     : migration-engine-cli fcbc4bb2d306c86c28014f596b1e8c7980af8bd4 (at node_modules/@prisma/engines/migration-engine-darwin)
Introspection Engine : introspection-core fcbc4bb2d306c86c28014f596b1e8c7980af8bd4 (at node_modules/@prisma/engines/introspection-engine-darwin)
Format Binary        : prisma-fmt fcbc4bb2d306c86c28014f596b1e8c7980af8bd4 (at node_modules/@prisma/engines/prisma-fmt-darwin)
Studio               : 0.329.0
@ianmartorell ianmartorell added the kind/bug A reported bug. label Jun 8, 2021
@janpio janpio added topic: client types Types in Prisma Client tech/typescript Issue for tech TypeScript. bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. team/client Issue for team Client. labels Jun 8, 2021
@pantharshit00
Copy link
Contributor

Hey @ianmartorell

Can you please try this again with the latest version ? 2.13.1 is very old now so this might be already fixed.

If you can still reproduce this, please also share the schema of OrderItem model as it is being used in the reproduction call here.

@pantharshit00 pantharshit00 added bug/0-unknown Bug is new, does not have information for reproduction or reproduction could not be confirmed. and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Jun 9, 2021
@ianmartorell
Copy link
Author

ianmartorell commented Jun 26, 2021

Hi @pantharshit00, I have updated Prisma to the latest version (2.25.0) and the issue persists.

Since I have both a model called Product and ProductUpdate, there's a conflict in the generated types, and typescript expects the type of the includes of productUpdate to be ProductInclude, instead of ProductUpdateInclude.

Screenshot 2021-06-26 at 19 26 16

Screenshot 2021-06-26 at 19 27 44

Here's the schema of OrderItem, removing a bunch of non-relevant fields:

model OrderItem {
  id                         String          @id @default(uuid())
  quantity                   Int             @default(1)
  createdAt                  DateTime        @default(now())
  updatedAt                  DateTime        @updatedAt
  productUpdateId            String
  productUpdate              ProductUpdate   @relation(fields: [productUpdateId], references: [id])
  orderId                    String
  order                      Order           @relation(fields: [orderId], references: [id])
  orderItemStatusId          String
  orderItemStatus            OrderItemStatus @relation(fields: [orderItemStatusId], references: [id])
}

ianmartorell added a commit to ianmartorell/prisma-issue-7518 that referenced this issue Jun 26, 2021
ianmartorell added a commit to ianmartorell/prisma-issue-7518 that referenced this issue Jun 26, 2021
ianmartorell added a commit to ianmartorell/prisma-issue-7518 that referenced this issue Jun 26, 2021
@ianmartorell
Copy link
Author

@janpio janpio added bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. and removed bug/0-unknown Bug is new, does not have information for reproduction or reproduction could not be confirmed. labels Jun 29, 2021
@ianmartorell
Copy link
Author

@janpio @pantharshit00 Any idea when this could be looked over? If it's gonna take a while, I'll dedicate some time to rework our current Prisma schema to avoid this issue.

@janpio
Copy link
Member

janpio commented Jul 16, 2021

Next step would be for @pantharshit00 to get a confirmed reproduction, then someone from the developers would take a look and hopefully directly be able to reproduce as well, and then find a fix. Hard to say how complex that will be, before it actually happens - sorry.

@ianmartorell
Copy link
Author

No problem, thank you for the quick reply :)

@pantharshit00
Copy link
Contributor

Sorry for the late reply here. This got buried in my notifications.

I can confirm the conflict and marking this as candidate.

@pantharshit00 pantharshit00 removed their assignment Aug 1, 2021
@pantharshit00 pantharshit00 added bug/2-confirmed Bug has been reproduced and confirmed. process/candidate and removed bug/1-unconfirmed Bug should have enough information for reproduction, but confirmation has not happened yet. labels Aug 1, 2021
@dpetrick dpetrick added topic: reserved words See https://www.prisma.io/docs/reference/api-reference/prisma-schema-reference#naming-conventions and removed process/candidate labels Aug 6, 2021
SevInf added a commit that referenced this issue Jul 12, 2023
In some cases, generated types for one of the models will clash with
generated types for another. In that case, we would generate duplicate
type.
This PR fixes this: approach is very different to namespaces proposal I
shared earlier. Turns out, most of the conflicts are solved by just
renaming default args types from `ModelArgs` to `ModelDefaultArgs`.
Deperecated alias for an old name would still be created if it does not
conflict.
Somewhat different case is `Payload` type. First, they are created at
top level of the file, rather then in `Prisma` namespace. @millsp and I
agreed that thouse types are not public, so it is safe to move them to
namespace and rename them. So, `UserPayload` would now become
`Prisma.$UserPayload`. This allows to both use `UserPayload` as a model
name as well as use `Batch` as a model name (we have built-in
`BatchPayload` type that is unrelated to aforementioned `Payload`
types).

Fix #19967
Fix #18902
Fix #16940
Fix #9568
Fix #7518
SevInf added a commit that referenced this issue Jul 12, 2023
In some cases, generated types for one of the models will clash with
generated types for another. In that case, we would generate duplicate
type.
This PR fixes this: approach is very different to namespaces proposal I
shared earlier. Turns out, most of the conflicts are solved by just
renaming default args types from `ModelArgs` to `ModelDefaultArgs`.
Deperecated alias for an old name would still be created if it does not
conflict.
Somewhat different case is `Payload` type. First, they are created at
top level of the file, rather then in `Prisma` namespace. @millsp and I
agreed that thouse types are not public, so it is safe to move them to
namespace and rename them. So, `UserPayload` would now become
`Prisma.$UserPayload`. This allows to both use `UserPayload` as a model
name as well as use `Batch` as a model name (we have built-in
`BatchPayload` type that is unrelated to aforementioned `Payload`
types).

Fix #19967
Fix #18902
Fix #16940
Fix #9568
Fix #7518
SevInf added a commit that referenced this issue Jul 18, 2023
In some cases, generated types for one of the models will clash with
generated types for another. In that case, we would generate duplicate
type.
This PR fixes this: approach is very different to namespaces proposal I
shared earlier. Turns out, most of the conflicts are solved by just
renaming default args types from `ModelArgs` to `ModelDefaultArgs`.
Deperecated alias for an old name would still be created if it does not
conflict.
Somewhat different case is `Payload` type. First, they are created at
top level of the file, rather then in `Prisma` namespace. @millsp and I
agreed that thouse types are not public, so it is safe to move them to
namespace and rename them. So, `UserPayload` would now become
`Prisma.$UserPayload`. This allows to both use `UserPayload` as a model
name as well as use `Batch` as a model name (we have built-in
`BatchPayload` type that is unrelated to aforementioned `Payload`
types).

Fix #19967
Fix #18902
Fix #16940
Fix #9568
Fix #7518
SevInf added a commit that referenced this issue Jul 18, 2023
In some cases, generated types for one of the models will clash with
generated types for another. In that case, we would generate duplicate
type.
This PR fixes this: approach is very different to namespaces proposal I
shared earlier. Turns out, most of the conflicts are solved by just
renaming default args types from `ModelArgs` to `ModelDefaultArgs`.
Deperecated alias for an old name would still be created if it does not
conflict.
Somewhat different case is `Payload` type. First, they are created at
top level of the file, rather then in `Prisma` namespace. @millsp and I
agreed that thouse types are not public, so it is safe to move them to
namespace and rename them. So, `UserPayload` would now become
`Prisma.$UserPayload`. This allows to both use `UserPayload` as a model
name as well as use `Batch` as a model name (we have built-in
`BatchPayload` type that is unrelated to aforementioned `Payload`
types).

Fix #19967
Fix #18902
Fix #16940
Fix #9568
Fix #7518
@Jolg42 Jolg42 added this to the 5.1.0 milestone Jul 18, 2023
@SevInf
Copy link
Contributor

SevInf commented Jul 18, 2023

This issue is now fixed and the fix will be published as a part of 5.1 release. If you'd like to verify fix earlier, you can use dev snapshot 5.1.0-dev.24. We don't recommend using it in production, but it should be enough to verify the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed Bug has been reproduced and confirmed. kind/bug A reported bug. team/client Issue for team Client. tech/typescript Issue for tech TypeScript. topic: client types Types in Prisma Client topic: reserved words See https://www.prisma.io/docs/reference/api-reference/prisma-schema-reference#naming-conventions topic: type-clash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants