MichalLytek / typegraphql-prisma

Prisma generator to emit TypeGraphQL types and CRUD resolvers from your Prisma schema
https://prisma.typegraphql.com
MIT License
889 stars 113 forks source link

Generating correct return types for prisma actions resolver methods #107

Open amosbastian opened 3 years ago

amosbastian commented 3 years ago

Describe the Bug

For example, the generated aggregate return type is incorrect. Currently it could be something like:

AggregatePost {
    count: PostCountAggregate | null;
    avg: PostAvgAggregate | null;
    sum: PostSumAggregate | null;
    min: PostMinAggregate | null;
    max: PostMaxAggregate | null;
}

But when typing ctx correctly instead of as any, thus correctly typing prisma, using it like so

@Query(() => AggregatePost, { nullable: false })
async aggregatePost(
  @Ctx() { prisma }: Context,
  @Info() info: GraphQLResolveInfo,
  @Args() args: AggregatePostArgs,
): Promise<AggregatePost> {
  const aggregation = await prisma.post.aggregate({
    ...args,
    ...transformFields(graphqlFields(info)),
  });

  return aggregation;
}

results in the following error:

Type 'GetPostAggregateType<{ where?: PostWhereInput | undefined; orderBy?: PostOrderByInput[] | undefined; cursor?: PostWhereUniqueInput | undefined; take?: number | undefined; skip?: number | undefined; }>' is missing the following properties from type 'AggregatePost': count, avg, sum, min, max

To Reproduce

  1. Generate resolvers
  2. Type ctx properly instead of as any, or correctly type return type of getPrismaFromContext
  3. Return type does not match

Same happens for generated relation resolvers (e.g. author of post) since Prisma's findUnique can return null. Also groupBy seems to have mismatched typing as well.

Expected Behavior

The return type of the generated resolvers should match what is returned by the Prisma client.

Environment (please complete the following information):

Additional Context

I am probably not using typegraphql-prisma as most people because I am mostly using the generated types, models and helpers and using them to create my own resolvers instead of importing the generated resolvers directly, so maybe my use case is not relevant to others but I hope this is useful nevertheless.

MichalLytek commented 3 years ago

I am probably not using typegraphql-prisma as most people because I am mostly using the generated types, models and helpers and using them to create my own resolvers instead of importing the generated resolvers directly, so maybe my use case is not relevant to others but I hope this is useful nevertheless.

Definitely 😉

Improving interoperability of types is of course on my todo list, however right now I prefer to focus on functionality for the common cases of using generated artifacts.

I would need to start by importing the Prisma type or even allowing providing own context type including Prisma, which should allow me to easier spot such places.

However I won't plan to focus on that too much, as transformFields and graphqlFields are not strongly typed, that's why Prisma type inference mechanism doesn't play well with that. You should try to write aggregatePost resolver on your own to see what I mean 😜

JClackett commented 2 years ago

Commenting here to also say that I, like @amosbastian, never use the generated resolvers as there's rarely a time where its ever just CRUD for me, I'm a bit surprised people are able to just use the generated resolver code tbh. For basic projects I can see how it can be quick to prototype.

For example, a simple login service:

  async login(data: LoginInput): Promise<User> {
    const user = await prisma.user.findUnique({ where: { email: data.email } })
    if (!user) throw new UserInputError("Incorrect email or password")
    const isValidPassword = await bcrypt.compare(data.password, user.password)
    if (!isValidPassword) throw new UserInputError("Incorrect email or password")
    return user
  }

I now can't use the generated User type as it says that the _count property is missing. How are people building apps where just CRUD is possible?

In this page in the docs, won't this give an TS error now? because prisma isn't returning the _count property.

I understand that matching the prisma + GraphQL types is hard, but it seems like making the _count property still nullable, at least lets the custom resolvers still be useable. Not typing the return function or going around putting Omit<User, "_count"> everywhere is really not ideal.

Just my thoughts, I do really appreciate all the work you're doing on this @MichalLytek!!

MichalLytek commented 2 years ago

@JClackett _count issue will be fixed soon. The main problem in this issue is with nullability and omited fields.

Also some of advanced Prisma things like count or even aggregate, because of the way it structures the output data, requires some dynamic operations which are not type-safe and silenced by any in generated code.

But for resolvers is not a big deal cause we don't reuse them. Only args, inputs and outputs are shared with custom resolvers.

How are people building apps where just CRUD is possible?

You can compose your custom resolvers with the generated ones or even reuse args, inputs and models to add some custom logic or behavior to Prisma actions like create. That's the design goal for typegraphql-prisma.