prisma-labs / graphqlgen

⚙️ Generate type-safe resolvers based upon your GraphQL Schema
MIT License
818 stars 54 forks source link

Make createUser name argument default null #397

Closed nunovieira closed 5 years ago

nunovieira commented 5 years ago

This is the simplest fix for #396 I came up with. But maybe name should be required, as in flow-yoga template?

jasonkuhrt commented 5 years ago

Hey @nunovieira thanks for this! It looks like our improved type generation in 0.5.0 is catching subtle type errors previously latent.

We ought to have a test suite for avoiding breaking the templates again in the future like that.

You're fix is certainly reasonable but there are alternatives to changing the model types, such as:

diff --git a/src/schema.graphql b/src/schema.graphql
index 31321e1..ca6e4d8 100644
--- a/src/schema.graphql
+++ b/src/schema.graphql
@@ -5,7 +5,7 @@ type Query {
 }

 type Mutation {
-  createUser(name: String): User!
+  createUser(name: String = null): User!
   createDraft(title: String!, content: String!, authorId: ID!): Post!
   deletePost(id: ID!): Post
   publish(id: ID!): Post
diff --git a/src/resolvers/Mutation.ts b/src/resolvers/Mutation.ts
index a66005f..b0bc1f9 100644
--- a/src/resolvers/Mutation.ts
+++ b/src/resolvers/Mutation.ts
@@ -1,7 +1,7 @@
 import { MutationResolvers } from '../generated/graphqlgen'

 export const Mutation: MutationResolvers.Type = {
-  createUser: (parent, { name }, ctx) => {
+  createUser: (parent, { name = null }, ctx) => {
     const id = ctx.data.idProvider()
     const newUser = { id, name, postIDs: [] }
     ctx.data.users.push(newUser)

I think using null default in the schema might be a nice way to solve whilst showing off that feature too. What do you think?

nunovieira commented 5 years ago

I didn't knew that feature! We could do that. I leave that decision to you (the maintainers), but I think we should have the same schema in both templates.

nunovieira commented 5 years ago

I've amended the commit with your suggestion. I didn't update the flow-yoga template because I got errors with name nullable:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/data.js:31:36

Cannot assign object literal to data because:
 • string [1] is incompatible with null [2] in property name of array element of property users.
 • string [3] is incompatible with null [2] in property name of array element of property users.

     src/data.js
 [1]  5│   { id: '1', name: 'Alice', postIDs: ['3', '4'] },
 [3]  6│   { id: '2', name: 'Bob', postIDs: [] },
       :
     28│   return `${idCount++}`
     29│ }
     30│
     31│ export const data: Data = { posts, users, idProvider }
     32│

     src/types.js
 [2]  9│   name: string | null;

Found 2 errors

I'm not that comfortable with flow to make that change.