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

Always set rejectOnNotFound to false #98

Closed
jpulec opened this issue Jul 9, 2021 · 10 comments · Fixed by #135
Closed

Always set rejectOnNotFound to false #98

jpulec opened this issue Jul 9, 2021 · 10 comments · Fixed by #135
Assignees
Labels
type/feat Add a new capability or enhance an existing one

Comments

@jpulec
Copy link

jpulec commented Jul 9, 2021

Description

It seems that there's some sort of interaction between setting rejectOnNotFound: true for findUnique globally and how nexus-prisma tries to resolve relations. Because nexus-prisma uses prisma's fluent API, it performs a findUnique for the parent model, and then tries to fetch the relation via the fluent API, which throws a Not Found error if the relation is null.

// schema.prisma

model User {
  id @id
}

model Post {
   id @id
   userId  String? @map("user_id")
   user  User? @relation("user_id")
}
import { objectType } from "nexus";

import { Post } from "nexus-prisma";

export const Post = objectType({
  name: "Post",
  definition(t) {
    t.nonNull.id("id");
    t.field(Post.user);
  }
});
// lib/prisma.ts
import { PrismaClient } from "@prisma/client";

const  prisma = new PrismaClient({
  rejectOnNotFound: {
    findUnique: true,
  },
});

The above causes nexus-prisma to try to run prisma.post.findUnique({ where: { id: <ID> } }).user() which will throw a not found error if it is null.

Happy to provide any more explanation.

@jpulec jpulec added the type/bug Something is not working the way it should label Jul 9, 2021
@jasonkuhrt
Copy link
Contributor

@jpulec can you clarify what you expect to happen here? Are you saying you don't want the projected field to throw? More specifically: Even though you want a rejectOnNotFound policy, you don't want this policy to apply to fields projected by nexus-prisma?

@jasonkuhrt jasonkuhrt added the needs/discussion Something needs deciding, new info changes issue scope/spec, unforeseen challenges, etc. label Aug 26, 2021
@jpulec
Copy link
Author

jpulec commented Aug 26, 2021

I did a little digging into this, and I actually believe it's an issue with prisma. When you set rejectOnNotFound globally for findUnique, and use the fluent API, prisma rejects if either the findUnique() call returns null OR if the fluent relation call returns null. This seems like really odd behavior to me, since I don't believe I can disable throwing if I expect the fluent relationship might be null. i.e. the following doesn't work.

import { PrismaClient } from "@prisma/client";

const  prisma = new PrismaClient({
  rejectOnNotFound: {
    findUnique: true,
  },
});

await prisma.post.findUnique({ where: { id: 'test-id' } }).user({ rejectOnNotFound: false }); // This isn't a real option

And even if the above did work, I'd still need a way to pass that option to nexus-prisma to indicate not to reject on the fluent API call.

I really might need to take this up with the prisma team as well, since I think they'd need to make some changes first. But the issue overall is that if you choose to globally set rejectOnNotFound and you expect to have some nullable relations, using the projections from nexus-prisma isn't really an option.

I think ideally, prisma shouldn't treat fluent api calls that are chained from .findUnique() as having the same rejectOnNotFound value. Again, this is probably something I need to raise with the prisma team first.

@jasonkuhrt
Copy link
Contributor

jasonkuhrt commented Aug 27, 2021

I think ideally, prisma shouldn't treat fluent api calls that are chained from .findUnique() as having the same rejectOnNotFound value.

Could you explain a bit more your rationale for why the behaviour between the two should diff?

(I work at Prisma by the way)

@jpulec
Copy link
Author

jpulec commented Aug 27, 2021

Sure. I think the challenge is that I can't override the calls separately, which is more of an issue when setting rejectOnNotFound globally. Regularly, if I set rejectOnNotFound globally, I can override it on a case-by-case basis, which works well.

However, I find that it's pretty common to expect a .findUnique() to return a value, but the fluent call to not return a value, especially for the case of a 1-to-1 relationship.

The way I think of it, doing the fluent API call is conceptually to me a second lookup (and sometimes is actually a separate query, if I'm not mistaken?), which means I have different expectations around whether or not I'm okay with it being nullable.

Please correct me if I'm wrong, but I also believe if I did the following:

await prisma.post.findUnique({ where: { id: 'fake-id'}, rejectOnNotFound: false }).user();`

and the result was null, I wouldn't be able to determine if the result was null because there was no post, or because there was no user. Is that correct?

@P4sca1
Copy link

P4sca1 commented Aug 29, 2021

From my understanding, if you provide a nullable relation (like in this case User?), nexus should not throw when post.user() does not exist, but return null.
For this to work, rejectOnNotFound: false needs to be explicitly set to false for nullable relations using the fluent API, but NOT on the parent type (which seems to be not possible at the moment).

@jasonkuhrt
Copy link
Contributor

Here's How I'm seeing the mapping:

(DB)                            (API)
Prisma                          GraphQL
------                          -------

nullable field relation     ->  nullable field relation

model A {                       type A {
  foo Foo?                        foo: Foo
}                               }

non-nullable field relation ->  non-nullable field relation

model A {                       type A {
  foo Foo                         foo: Foo!
}                               }

list field relation         ->  non-nullable list non-nullable field relation

model A {                       type A {
  foos Foo[]                      foo: [Foo!]!
}                               }

I think NP should ignore rejectOnNotFound. Ignoring means always having it set to false on queries it sends.

Does this make sense?

@jasonkuhrt jasonkuhrt added type/feat Add a new capability or enhance an existing one and removed type/bug Something is not working the way it should labels Aug 30, 2021
@jpulec
Copy link
Author

jpulec commented Aug 30, 2021

I think that's probably a good start. Maybe it's worth making it configurable, but I think just always setting rejectOnNotFound to false would be better default behavior.

@jasonkuhrt jasonkuhrt changed the title Nullable Relation with Global rejectOnNotFound Set Fails Always set rejectOnNotFound to false Aug 30, 2021
@jasonkuhrt jasonkuhrt self-assigned this Aug 30, 2021
@jasonkuhrt jasonkuhrt removed the needs/discussion Something needs deciding, new info changes issue scope/spec, unforeseen challenges, etc. label Aug 30, 2021
jasonkuhrt added a commit that referenced this issue Aug 30, 2021
@jasonkuhrt
Copy link
Contributor

I started the PR with updated docs. Feedback welcome https://github.com/prisma/nexus-prisma/tree/feat/reject-on-not-found-false#projecting-nullability.

@jasonkuhrt
Copy link
Contributor

jasonkuhrt commented Aug 30, 2021

Maybe it's worth making it configurable

@jpulec Please bring a use-case in the future when you have one.

Also remember you can override the NP resolvers with your own logic fwiw.

@rostislav-simonik-nexus-prisma-admin
Copy link
Collaborator

🎉 This issue has been resolved in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feat Add a new capability or enhance an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants