fdarian / prisma-generator-drizzle

A Prisma generator for generating Drizzle schema with ease
129 stars 11 forks source link

In one to one relations, the side without foreign key shouldn't have relation opts #69

Closed jansedlon closed 2 months ago

jansedlon commented 3 months ago

TL;DR

This is correct

export const oneToOneBsRelations = relations(oneToOneBs, (helpers) => ({
    a: helpers.one(oneToOneAs)
);

export const oneToOneAsRelations = relations(oneToOneAs, (helpers) => ({
    b: helpers.one(oneToOneBs, {
        relationName: 'OneToOne_AToOneToOne_B',
        fields: [ oneToOneAs.bId ],
        references: [ oneToOneBs.id ] })
    })
);

This is not

export const oneToOneBsRelations = relations(oneToOneBs, (helpers) => ({
    a: helpers.one(oneToOneAs, {
        relationName: 'OneToOne_AToOneToOne_B',
        fields: [oneToOneBs.id],
        references: [oneToOneAs.bId] })
    })
  // This whole opts object shouldn't be there
);

export const oneToOneAsRelations = relations(oneToOneAs, (helpers) => ({
    b: helpers.one(oneToOneBs, {
        relationName: 'OneToOne_AToOneToOne_B',
        fields: [ oneToOneAs.bId ],
        references: [ oneToOneBs.id ] })
    })
);

Hi, if you have one to one relation (A, B) and the foreign key is in table A, the one relation on the other side shouldn't have any opts.

If it does, drizzle query with with does not infer it as an optional relation.

The best showcase is if you open this repo, hover your mouse over result in packages/usage/tests/shared/testOneToOne.ts

image

You can see that the a property is not nullable even though it should be.

From my understanding, if you pass any opts into one relation function, it's then inferred as required when using drizzle query.

https://orm.drizzle.team/docs/rqb#one-to-one

I found that the issue is here https://github.com/farreldarian/prisma-generator-drizzle/blob/ed849c2e304a4bdf9b0daaa66d4c4e01e46542c2/packages/generator/src/lib/adapter/declarations/generateTableRelationsDeclaration.ts#L126

In this case the whole object shouldn't be passed there. I don't really understand the black magic of that code, but in this case, it shouldn't be there :D

fdarian commented 3 months ago

From my understanding, if you pass any opts into one relation function, it's then inferred as required when using drizzle query.

Yeah it was tricky since I didn't had the absolute reference when building the relational generator, whether defining or not defining opts determines the optionality. I thought it was a bug in drizzle.

I think we can proceed following the current (your) finding for the optionality rule.

Expect the fix at last this week.

UPDATE: optionality doesn't matter, it should always be nullable

fdarian commented 3 months ago

Ah, I now understand the reason for including all these references.

Given the following schema:

model Disambiguating_Transfer {
  id          String               @id
  salePayment Disambiguating_Sale? @relation("Disambiguating_Sale_payment")
  saleTax     Disambiguating_Sale? @relation("Disambiguating_Sale_tax")
}

model Disambiguating_Sale {
  id        String                  @id
  payment   Disambiguating_Transfer @relation("Disambiguating_Sale_payment", fields: [paymentId], references: [id])
  paymentId String                  @unique
  tax       Disambiguating_Transfer @relation("Disambiguating_Sale_tax", fields: [taxId], references: [id])
  taxId     String                  @unique
}

To determine Transfer.salePayment and Transfer.saleTax (table name is simplified), ideally, we could just specify the relationName, but how can we achieve that in drizzle?

Reference: https://github.com/farreldarian/prisma-generator-drizzle/actions/runs/9650395882/job/26615929983#step:6:212

Ideally, but doesn't work CleanShot 2024-06-25 at 01 29 59@2x

Specifying the name, field, and references, makes the field required CleanShot 2024-06-25 at 01 33 25@2x

fdarian commented 3 months ago

Posted in drizzle's help

https://discord.com/channels/1043890932593987624/1254869164607143998

jansedlon commented 3 months ago

Nice, good observations, thank you! Luckily i don't have this use case in my code and i haven't encountered that.

fdarian commented 3 months ago

I think we can proceed with the no-opts option, and utilizing opts for cases that have disambiguating relationship. This should cover most cases

jansedlon commented 3 months ago

Sounds good