Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Nullable field causing type assignment error #50

Closed
JasCodes opened this issue Sep 28, 2018 · 34 comments
Closed

Nullable field causing type assignment error #50

JasCodes opened this issue Sep 28, 2018 · 34 comments
Assignees

Comments

@JasCodes
Copy link

JasCodes commented Sep 28, 2018

Hey @divyenduz @timsuchanek
Steps to reproduce

Use below schema.graphql and yield fresh interfaces and scaffold.

type Query {
  me: User
}

type Mutation {
  signup(name: String!, email: String!, password: String!): AuthPayload!
  login(email: String!, password: String!): AuthPayload!
}

type AuthPayload {
  token: String!
  user: User!
}

# The `User` type is a reduced version ("mask") of the `User` type from the data model (and database schema).
# It does not expose the `createdAt`, `updatedAt` and `password` fields.
type User {
  id: ID!
  email: String!
  name: String
}

Following type error is produced
screenshot 9

{
	"resource": "/c:/Users/jas/play/prt2/src/resolvers/User.ts",
	"owner": "typescript",
	"code": "2322",
	"severity": 8,
	"message": "Type '(parent: UserParent) => string | undefined' is not assignable to type '(parent: UserParent, args: {}, ctx: Context, info: GraphQLResolveInfo) => string | Promise<string | null> | null'.\n  Type 'string | undefined' is not assignable to type 'string | Promise<string | null> | null'.\n    Type 'undefined' is not assignable to type 'string | Promise<string | null> | null'.",
	"source": "ts",
	"startLineNumber": 13,
	"startColumn": 3,
	"endLineNumber": 13,
	"endColumn": 7,
	"relatedInformation": [
		{
			"startLineNumber": 138,
			"startColumn": 5,
			"endLineNumber": 138,
			"endColumn": 9,
			"message": "The expected type comes from property 'name' which is declared here on type 'Type<TypeMap>'",
			"resource": "/c:/Users/jas/play/prt2/src/generated/resolvers.ts"
		}
	]
}
@JasCodes
Copy link
Author

JasCodes commented Sep 28, 2018

Cause of type error:
Interfaces use null but resolver using undefined; for making property nullable.

I will be adding PR to this thread that will change interface to use undefined instead of null.
Although you the choose reverse if you want to.

Thx

@JasCodes
Copy link
Author

screenshot 10
@divyenduz Also getting can't find T in interfaces gen

Test based on basic schema

type Query {
  id: ID!
  custom_required: Number!
  custom_nullable: Number
  custom_array_nullable: [Number]
  custom_array_required: [Number]!
  custom_with_arg(id: Int!): Number!
  custom_with_custom_arg(id: Number!): Number!
  scalar_required: Boolean!
  scalar_nullable: Boolean
  scalar_array_nullable: [Boolean]
  scalar_array_required: [Boolean]!
  scalar_with_arg(id: Int!): Boolean!
  scalar_with_custom_arg(id: Number!): Boolean!
  # commented_field: Boolean!
}

type Number {
  id: ID
  value: Float
}

@JasCodes
Copy link
Author

@divyenduz I think you should research code from https://github.com/dotansimha/graphql-code-generator

@divyenduz
Copy link
Contributor

@Jas99 : Can you please share the version of your graphql-resolver-codegen by using graphql-resolver-codegen --version

@JasCodes
Copy link
Author

JasCodes commented Sep 29, 2018

Hey @divyenduz
I was testing against the basic schema from fixtures in this repo.
The resolver generation issue might be related to error in schema itself.

custom_with_custom_arg(id: Number!): Number!
scalar_with_custom_arg(id: Number!): Boolean!

I don't think one can use types as arguments, doesn't it has to be input type?

PS: using 0.2.4

@divyenduz
Copy link
Contributor

Yes, it has to be an input type. This looks like a bug.

@divyenduz divyenduz self-assigned this Sep 29, 2018
JasCodes added a commit to JasCodes/graphql-resolver-codegen that referenced this issue Sep 30, 2018
@kevinmarrec
Copy link

@divyenduz Any Update on this issue ?
@Jas99 proposed #52

@kevinmarrec
Copy link

And this issue only happens when having strictNullChecks: true in tsconfig.json, if I'm right.

@JasCodes
Copy link
Author

JasCodes commented Oct 9, 2018

@kevinmarrec No that was not the issue. Generated interfaces returns Null but the scaffold is generating undefined type instead of null; which causes type mismatch.

@kevinmarrec
Copy link

kevinmarrec commented Oct 9, 2018

Oh yeah I got confused 😅, but anyway the error which allowed you to see the issue could only happen with the strictNullChecks: true option (or strict: true). Cause without it TypeScript doesn't seem to care if your resolver return null instead of undefined or vice-versa.

So the PR is fixing the scaffolding and when we want to do things by ourselves it's about replacing variable?: Type by variable: Type | null right ? If I correctly understood what you did.

@JasCodes
Copy link
Author

JasCodes commented Oct 9, 2018

@kevinmarrec Right on.. actually for some weird reason @divyenduz intentionally introduced undefined which might not be inline with graphqljs implementation.

@divyenduz
Copy link
Contributor

@Jas99 : Thanks! that makes sense, can you please create a minimal reproduction project with strictNullChecks: true

In the mean time, can you also please create an issue for the input issue, hopefully with a reproduction 🙏

I will try to get both these resolved ASAP. Thanks!

@JasCodes
Copy link
Author

JasCodes commented Oct 9, 2018

@divyenduz Buddy strick mode is true by default which means strickNullCheckts is also true by default and the issue with type-mismatch was not edge case.

typescript-graphql-auth in prisma-examples has user with nullable property.
https://github.com/prisma/prisma-examples/blob/master/typescript-graphql-auth/src/resolvers/User.ts

If you lint it; you will see the issue.

screenshot 10

Please re-read the very first comment. I have reattached the pic.

PS: My pr #52 fixes this. I will merge PR with recent update from master.

Thx

@JasCodes JasCodes mentioned this issue Oct 9, 2018
@kevinmarrec
Copy link

kevinmarrec commented Oct 10, 2018

@Jas99 Removing the ? and replacing by | null in [Model]Parent interfaces seems to break Types for Mutations and Queries :/

screenshot_2
screenshot_1

cause of

screenshot_2

@kevinmarrec
Copy link

kevinmarrec commented Oct 10, 2018

@Jas99 As far as I know, the problem is :

Typescript ? means | undefined

But in GraphQL Language it means that the field can return a "nullable" so the type needs to be | null

@divyenduz
Copy link
Contributor

@Jas99 @kevinmarrec : Can anyone of you prepare a minimal reproduction repository? Currently, I just have some code + an image. Real code would help 🙏

@JasCodes
Copy link
Author

@kevinmarrec I personally use https://www.npmjs.com/package/@jas99/graphql-resolver-codegen
Generate both interfaces and scaffold with this variant; its tested by me.

@kevinmarrec
Copy link

@divyenduz https://github.com/kevinmarrec/typescript-prisma-minimal

  1. You should have a TS lint error in resolvers/User.ts
  2. Replace notRequiredField?: string by notRequiredField: string | null
  3. Problem in 1) solved BUT new error in Query.ts

@JasCodes
Copy link
Author

@kevinmarrec
Copy link

@Jas99 On typescript-graphql-auth if I replace ? by | null in Parent Models, I get TS issues around Queries/Mutations using this model as return value (or in a payload which contains this model).

@kevinmarrec
Copy link

And typescript-graphql-auth doesn't have the strictNullChecks: true in tsconfig.json

@JasCodes
Copy link
Author

JasCodes commented Oct 10, 2018

@kevinmarrec strictNullChecks is true by default

@kevinmarrec
Copy link

kevinmarrec commented Oct 10, 2018

@Jas99 Not for me

@JasCodes
Copy link
Author

JasCodes commented Oct 10, 2018

@kevinmarrec when you do tsc --init; can you see "strict": true, thr?
Which editor do you use?

@kevinmarrec
Copy link

@Jas99 Well in my project I have a custom tsconfig.json, as typescript-graphql-auth does. Maybe tsc --init generate a filed with strict: true but it doesn't mean that strict has true as default value with an empty tsconfig.json

@kevinmarrec
Copy link

But anyway with strict: true you should have issues in Queries/Mutations, using your solution with | null

@JasCodes
Copy link
Author

@Jas99 Maybe thr are new changes; I will look into it. Meanwhile instead of graphql-resolver-codegen use @jas99/graphql-resolver-codegen and let me know if even this is breaking; caz I have tested this to be working.

@kevinmarrec
Copy link

kevinmarrec commented Oct 10, 2018

@divyenduz I tried locally to replace all null by undefined in src/generated/resolvers.ts, everything seems to be alright => 0 issues with TypeScript + Everything works fine executing GraphQL queries when requesting required/not-required fields. (GraphQL handle by itself undefined values to result as null in data responses)

Maybe should we replace all null by undefined around resolver generation ? Do you have any thoughts about not doing that ?

@JasCodes
Copy link
Author

@kevinmarrec Check conversation at #51

@kevinmarrec
Copy link

@Jas99 Checked but I still don't know what is the problem ? It seems to be a choice by opinion and not a followed directive provided by GraphQL-Js (if i'm wrong please give me some references to read or code to know what's going on between using undefined or null)

@kevinmarrec
Copy link

kevinmarrec commented Oct 10, 2018

Prisma Client itself generates Model Nodes (UserNode for example) with undefined and not null, that's what cause typescript issues around mutations and queries as they must return something like Nodes Types but fail cause our Model Parent (UserParent refering to UserNode) results in type mismatching somewhere around Resolver definitions. (undefined !== null)

@schickling
Copy link
Contributor

I assume this has been fixed by the latest version. Please open a new issue if this problem still occurs.

@cyrus-za
Copy link

Check #278

@avatsaev
Copy link

How do I update a date field to null or undefiend?

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

6 participants