Skip to content
This repository has been archived by the owner on Sep 2, 2022. It is now read-only.

Generated Prisma client TS typings incompatible with graphqlgen typings for optional fields #3621

Closed
nikolasburk opened this issue Nov 29, 2018 · 12 comments

Comments

@nikolasburk
Copy link
Member

When using the Prisma client together with graphqlgen in TS scrict mode, the generated typings for optional fields from the Prisma datamodel and the graphqlgen resolver arguments don't match. Example:

datamodel.prisma

type User {
  id: ID! @unique
  email: String! @unique
  name: String
  posts: [Post!]!
}

type Post {
  id: ID! @unique
  createdAt: DateTime!
  updatedAt: DateTime!
  published: Boolean! @default(value: "false")
  title: String!
  content: String
  author: User!
}

For example, content on Post is optional here. Now consider this GraphQL schema:

scalar DateTime

type Query {
  feed: [Post!]!
  filterPosts(searchString: String): [Post!]!
  post(id: ID!): Post
}

type Mutation {
  signupUser(email: String!, name: String): User!
  createDraft(title: String!, content: String, authorEmail: String!): Post!
  deletePost(id: ID!): Post
  publish(id: ID!): Post
}

type Post {
  id: ID!
  createdAt: DateTime!
  updatedAt: DateTime!
  published: Boolean!
  title: String!
  content: String
  author: User!
}

type User {
  id: ID!
  email: String!
  name: String
  posts: [Post!]!
}

The content field is optional in the createDraft mutation. Now, the generated resolver args from graphqlgen for the input to createDraft look like this:

export interface ArgsCreateDraft {
  title: string
  content: string | null
}

On the other hand, the generated PostCreateInput type for the createPost operation in the Prisma client API looks as follows:

export interface PostCreateInput {
  published?: Boolean;
  title: String;
  content?: String;
  author: UserCreateOneWithoutPostsInput;
}

content here is declared as an optional instead of string | null. As fas as I understand, the optional is interpreted as string | undefined. Now, when implementing the createDraft resolver like so:

createDraft: (parent, { title, content, authorEmail }, ctx) => {
  return ctx.prisma.createPost({
    title,
    content,
    author: {
      connect: {
        email: authorEmail,
      },
    },
  })
},

TypeScript throws this error in strict mode:

src/resolvers/Mutation.ts:15:7 - error TS2322: Type 'string | null' is not assignable to type 'string | undefined'.
  Type 'null' is not assignable to type 'string | undefined'.

15       content,
         ~~~~~~~

  src/generated/prisma-client/index.ts:282:3
    282   content?: String;
          ~~~~~~~
    The expected type comes from property 'content' which is declared here on type 'PostCreateInput'

src/resolvers/Query.ts:14:29 - error TS2345: Argument of type '{ where: { OR: ({ title_contains: string | null; } | { content_contains: string | null; })[]; }; }' is not assignable to parameter of type '{ where?: PostWhereInput | undefined; orderBy?: "id_ASC" | "id_DESC" | "createdAt_ASC" | "createdAt_DESC" |"updatedAt_ASC" | "updatedAt_DESC" | "published_ASC" | "published_DESC" | ... 4 more ... | undefined; ... 4 more ...; last?: number | undefined; }'.
  Types of property 'where' are incompatible.
    Type '{ OR: ({ title_contains: string | null; } | { content_contains: string | null; })[]; }' is not assignable to type 'PostWhereInput'.
      Types of property 'OR' are incompatible.
        Type '({ title_contains: string | null; } | { content_contains: string | null; })[]' is not assignable to type 'PostWhereInput | PostWhereInput[] | undefined'.
          Type '({ title_contains: string | null; } | { content_contains: string | null; })[]' is not assignable to type 'PostWhereInput[]'.
            Type '{ title_contains: string | null; } | { content_contains: string | null; }' is not assignable to type 'PostWhereInput'.
              Type '{ title_contains: string | null; }' is not assignable to type 'PostWhereInput'.
                Types of property 'title_contains' are incompatible.
                  Type 'string | null' is not assignable to type 'string | undefined'.
                    Type 'null' is not assignable to type 'string | undefined'.

 14     return ctx.prisma.posts({
                                ~
 15       where: {
    ~~~~~~~~~~~~~~
...
 24       },
    ~~~~~~~~
 25     })
    ~~~~~

As suggested by @Weakky here, we need to In order to fix this, we need to both fix prisma-client and graphqlgen to type all nullable input fields as field?: type | null.

@nikolasburk
Copy link
Member Author

I just found out that this problem also applies to the Flow client. This is the Flow error that's thrown:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/resolvers/Mutation.js:12:34

Cannot call ctx.prisma.createPost with object literal bound to data because:
 • null [1] is incompatible with string [2] in property content.
 • object literal [3] is incompatible with undefined [4] in property author.connect.

     src/resolvers/Mutation.js
       9│     })
      10│   },
      11│   createDraft: (parent, { title, content, authorEmail }, ctx, info) => {
      12│     return ctx.prisma.createPost({
      13│       title,
      14│       content,
      15│       author: {
 [3]  16│         connect: { email: authorEmail },
      17│       },
      18│     })
      19│   },
      20│   deletePost: (parent, { id }, ctx, info) => {
      21│     return ctx.prisma.deletePost({

     src/generated/graphqlgen.js
 [1] 249│   content: string | null;

     src/generated/prisma-client/index.js
 [2] 276│   content?: String;
        :
 [4] 287│   connect?: UserWhereUniqueInput;

@jasonkuhrt
Copy link
Contributor

jasonkuhrt commented Dec 12, 2018

Effect of ? on object keys

As fas as I understand, the optional is interpreted as string | undefined

There is a difference between e.g. a: undefined | 1 and a?: 1

$ ts-node -v

ts-node v7.0.1
node v10.11.0
typescript v3.2.2

$ ts-node -O '{"strict":true}'
type Args = { a: undefined | 1, b?: 2 }

let args: Args

args = {}
// error TS2741: Property 'a' is missing in type '{}' but required in type 'Args'

args = { a: undefined }
// ok
let { a, b } = args
console.log(a, b)
// undefined undefined

As you can see only the a: undefined | 1 variant guarantees that a key is present. But from a consumer's point of view, it does not matter.

null vs undefined in graphql

Consider this schema:

  type Message {
    content: String!
  }

  type Query {
    messages(foo: Boolean, bar: Boolean = null): [Message!]!
  }

In the following queries, observe the resolver args values:

query implicit {
  messages {
    content
  }
}
{ bar: null }
query explicit {
  messages(foo: null) {
    content
  }
}
{ foo: null, bar: null }

To summarize, we have the following graphql-to-typescript mapping:

GQL                  TS
a: String            a?: null | string
                      ^  ^
                      |  | 2. user may pass explicit null
                      | 1. user may pass nothing
                      
a: String = null     a: null | string
                        ^
                        | 1. user may pass explicit null
                        | 2. user may pass nothing, thus gql null default

Reconciling graphqlgen and prism

One thing I would suggest tweaking about the proposed fix is that graphql gen should be able to output either case stated above: a?: null | string or a: null | string. As long as nullable prisma client args have type a?: null | string the types will work.

@b33son
Copy link

b33son commented Dec 12, 2018

Excellent work. Hope this gets fixed soon.

@breytex
Copy link

breytex commented Dec 18, 2018

I have exactly the same problem.
Graphql-code-generator has an avoidOptionals setting for this.
https://graphql-code-generator.com/docs/plugins/typescript-common

Maybe something similar could be the solution here?

@jasonkuhrt
Copy link
Contributor

@breytex there is already a clean solution identified though please let us know if you identified any flaws!

@breytex
Copy link

breytex commented Dec 18, 2018

@jasonkuhrt do you mean this PR? prisma-labs/graphqlgen#329
I just found it.
Unfortunately I cant test wether this PR solves my problem or not, because npm install prisma/graphqlgen#1f8b347 gives me "Missing package name". prisma/graphqlgen doesn't have a name specified in the package.json. Any other way I can test this without monkey patching?

Thanks :)

@jasonkuhrt
Copy link
Contributor

prisma/graphqlgen is a yarn workspaces monorepo so I don't think your install approach there will work. I don't actually have much experience with yarn workspaces / learna / monorepos in general (was ramping up tonight with some reading! 😅) so can't give you a confident one-liner right now.

I think something like this could work:

  • fork repo
  • clone
  • checkout pr branch
  • yarn install
  • yarn link
  • in your other project yarn link graphqlgen

Anyway, hopefully the issue will be resolved soon. Also better project ci/cd is something that is on the roadmap I think prisma-labs/graphqlgen#87

@steida
Copy link

steida commented Jan 8, 2019

Related to prisma/prisma#3774 (comment)

@durdenx
Copy link

durdenx commented Jan 29, 2019

Will this issue be solved soon? If i understand, for the the moment we have to add a // @ts-ignore comment before the problematic lines?

@lnmunhoz
Copy link

lnmunhoz commented Feb 1, 2019

Also looking forward for this fix. Any updates?

Weakky referenced this issue in prisma-labs/yoga2 Feb 10, 2019
TS seems to fail finding a global TS interface when put in a hidden folder
Add "strict: false" to examples until this lands prisma/prisma#3621
Weakky referenced this issue in prisma-labs/yoga2 Feb 10, 2019
TS seems to fail finding a global TS interface when put in a hidden folder
Add "strict: false" to examples until this lands prisma/prisma#3621
@stale
Copy link

stale bot commented Mar 18, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status/stale Marked as state by the GitHub stalebot label Mar 18, 2019
@pantharshit00 pantharshit00 added bug/2-confirmed and removed status/stale Marked as state by the GitHub stalebot labels Mar 18, 2019
@pantharshit00
Copy link
Contributor

I am going to close this in favour of #3774 as it has a more recent discussion about this and it covers more generic use case.

@steida Yes, you can use those. I am personally disabling strictNullChecks in my tsconfig while the typings get fixed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants