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

Scalar lists must not be nullable #1943

Closed
timsuchanek opened this issue Feb 20, 2018 · 8 comments
Closed

Scalar lists must not be nullable #1943

timsuchanek opened this issue Feb 20, 2018 · 8 comments

Comments

@timsuchanek
Copy link
Contributor

When creating a scalar list field like this:

type Model {
  intList: [Int!]!
}

The resulting schema generated looks like this:
image

@danielkcz
Copy link
Contributor

danielkcz commented Mar 19, 2018

@timsuchanek Just encountered this and it's especially painful with Typescript generated bindings. If you can point me in the right direction where this is generated, I can give a stab at fixing it.

@danielkcz
Copy link
Contributor

danielkcz commented Mar 22, 2018

Ok, I think I've tracked the whole thing down to this point.

https://github.com/graphcool/prisma/blob/3fe3c11073f38bc23f631fd1438760db1a8c4b5b/server/servers/api/src/main/scala/com/prisma/api/schema/ObjectTypeBuilder.scala#L136

I don't know Scala, but this part seems to be ok. That would mean that isRequired on models.Field is somehow messed up, but I cannot find that class. I assume that whole com.prisma.shared is a private part? It should be a really simple fix with a huge relieve for me.

@marktani Can you perhaps help with this or ping someone who can? I am rather annoyed by this and I really want to put some effort into fixing it.

@marktani
Copy link
Contributor

Thanks a lot for your feedback, @FredyC. I think a great first step is to start out with a test case that fails. Then you can dive into the solution 🙂

From what I can tell, because of this inconsistent schema generation, your generated bindings accepts null for some fields but it shouldn't.Can you help me understand the exact troubles you are experiencing as a consequence? 🙂

Thanks!

@danielkcz
Copy link
Contributor

danielkcz commented Mar 23, 2018

@marktani Thank you for your response.

I would love to provide a test case, but since this is Scala issue, I am not sure how that's done in there. I've been looking into some present tests, but it's so far from Jest and my head exploded :)

Ultimately the issue bubbles up to this in resolvers. You can spot that I have to force type return value because of inconsistent typings. It doesn't feel like a huge issue here, but it's kinda giving up on static typing benefits here and unable to discover mistakes easily.

export const myContacts: Query.myContacts = async (_, __, ctx, info) => {
  const playerId = getPlayerId(ctx)
  const players = await ctx.db.query.players(
    { orderBy: 'createdAt_DESC', where: { id_not: playerId } },
    info,
  )
  return players as Player[] // <-- have to force type here
}

namespace Query {
  export type myContacts = (parent: any, args: {}, context: Context, info?: GraphQLResolveInfo | string) => Promise<Player[]>
}

export interface Player {
  id: ID_Output
  fights: Fight[] // <-- correctly required field
}

Those types are generated with my custom generator based on an assembly of datamodel.graphql and several other files representing public api of my application layer.


Now on the other side where Prisma generates typings for binding it has its own Player declaration which is wrong.

Given datamodel type...

type Player {
  id: ID! @unique
  fights: [Fight!]!
}

The generated graphql schema looks like this

type Player implements Node {
  id: ID!
  fights(usualInputVariables): [Fight!]  # <-- missing require flag
}

This issue is then transformed to Typescript (with prisma-ts binding generator). This type is then returned from db.query.players and due to differences, it cannot be returned from the resolver as is. I've tried manually fixing Prisma typing and with that, it works just fine without any typecasting.

export interface Player extends Node {
  id: ID_Output
  fights?: Fight[] // <-- wrong optional field
}

@stale
Copy link

stale bot commented Jun 27, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed 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 Jun 27, 2018
@marktani marktani removed the status/stale Marked as state by the GitHub stalebot label Jul 5, 2018
@Esxiel
Copy link
Contributor

Esxiel commented Sep 4, 2018

Sorry in advance to ask without adding much to the discussion.
Not sure if its the same thing, but I thought this was somewhat intended?
The boilerplates usually have a type User of
type User { id: ID! @unique email: String! @unique password: String! posts: [Post!]!
but the schema generated will have a createUser that does not require posts(specifically [Post!]). Also iirc, using posts: [Post!] or posts: [Post]! will not work.
This is intended, right..?

EDIT: However, when defining the client API on schema.graphql, a [String!]! will properly require a [String!]!, not just [String!]

@mavilein
Copy link
Contributor

mavilein commented Nov 1, 2018

@Esxiel : Yes this intended. We don't required the posts in the input because an empty list would also be a valid value in this case. Hence it does not add any guarantees about the number of elements in the relation. For single relation fields, e.g. post: Post!, it is a different story.

@mavilein
Copy link
Contributor

mavilein commented Nov 1, 2018

Closing because this is already implemented.

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