[Prisma plugin] Non-nullable type should not be allowed with one-to-one relation

[tictaqqn]

There is a bug when 1 to 1 relation is used in Prisma schema.

Example schema file:

model Post {
  id         String         @id @default(cuid())
  postDetail PostDetail?
}

model PostDetail {
  id     String  @id @default(cuid())
  postId String  
  post   Post    @relation(fields: [postId], references: [id]) @unique
}

Example output:

builder.prismaObject('Post', {
  fields: (t) => ({
    id: t.exposeID('id'),
    postDetail: t.relation('postDetail', { nullable: false }) // Only `nullable: true` should be allowed!!
  })
})

is allowed but nullable: false should not allowed here because the prisma client could return null for the relation postDetail.

[hayes]

This is an odd edge case, and I've had users with strong opinions on both sides. While technically I think this would be more correct, and Pothos generally tries to be as type-safe and strict as possible, there are also users who have nullable relations in their schemas they know will always be set and want to expose as non-null.

I've gone back and forth on the correct thing to do here a few times. Taking a step back, another principle in Pothos (which the prisma plugin does violate in a few ways) is that it should ultimately always be the Pothos code, not the underlying datasources that determine the shape of a schema. Generally speaking the schema should never change without changing pothos code. This is why the nullability of a relation shouldn't change the nullability of a field. On the other hand, having a type-error when the underlying data does not match the schema you are defining is what most closely aligns with pothos principles.

In pothos the default is to be non-nullable, which makes postDetail: t.relation('postDetail') a non-nullable field, erroring in this case is more complicated (it's doable, but the messages are not very intuitive). Pothos v4 may be changing the default nullability, which would help the default case, but this situation would still need to be accounted for.

I agree that this should result in an Error, but the implementation needs to account for a few things:

• This needs to work with defaults in a few ways
  • should work consistently regardless of if fields are nullable by default or not
  • the error message should be clear, even if its a case where the nullable option (or the whole options object) is omitted
  • t.expose has similar issues, and until recently had very hard to decipher error messages, which were improved a bit recently
• There needs to be an easy way to overwrite this
  • Some sort of option to force non-null for nullable fields in schemas
  • Potentially a way to customize the error thrown if this forcing fails

I'm open to suggestions/PRs on how to handle this, unfortunately, I don't think something as simple as just making nullable: false error will be sufficient, as there are existing uses of the current behavior that needs to be accounted for.

[tictaqqn]

This is an odd edge case, and I've had users with strong opinions on both sides. While technically I think this would be more correct, and Pothos generally tries to be as type-safe and strict as possible, there are also users who have nullable relations in their schemas they know will always be set and want to expose as non-null.

I understand that there are indeed users who have nullable relations in their schemas that they know will always be set and want to expose as non-null.

• There needs to be an easy way to overwrite this
  • Some sort of option to force non-null for nullable fields in schemas
  • Potentially a way to customize the error thrown if this forcing fails

I see. I come up with options like below:

• Calling a resolver to handle the situation when the prisma client returns null.

postDetail: t.relation('postDetail', { nullable: false, resolveNull: () => {
  throw new Error('Message!')
} })

• Having an option like allowExposeNullableColumnAsNonNullable or forceNonNullable in the relation itself.

postDetail: t.relation('postDetail', { forceNonNullable: true })

But, the implementation may cause error messages with difficulty in understanding.

[hayes]

this may be a good candidate for #279, I have been working on v4 for a while, there are a couple open questions I need to resolve to actually get it released, but this issue feels like a good thing to add there