typegql icon indicating copy to clipboard operation
typegql copied to clipboard

make InputField nullable by default

Open capaj opened this issue 6 years ago • 2 comments

Field is nullable by default, whereas InputField is not. This is inconsistent and also it doesn't make much sense- at least in the API I am building. We have quite a few GQL calls like this:

 mutation {
                        user(id: ${id}) {
                          patch(input: {organisation_role: "role"}) {
                            organisation_role
                          }
                        }
                      }

now if I wanted to edit another field on my user, for example email:

 mutation {
                        user(id: ${id}) {
                          patch(input: {email: "[email protected]"}) {
                            email
                          }
                        }
                      }

so what I end up doing is marking all of my input fields on User entity as isNullable anyway. I've got the alias I've made, but it would make it easier if typegql had input fields as nullable by default.

What do you think @pie6k ?

capaj avatar Jun 05 '18 08:06 capaj

I'll think about it, but since I'm thinking now about unifing input and output fields into one universal one anyway, it's loosing it's point a bit so I'll defer decision here.

ps. consider using graphql variables instead of putting your values like this into query string. It's perfect opportunity for having sql injection in your api server.

mutation MyMutatiion($userId: ID!) {
  user(id: $userId) {
    patch(input: { organisation_role: "role" }) {
      organisation_role
    }
  }
}

pie6k avatar Jun 05 '18 09:06 pie6k

Sounds good :+1: I'll keep this open because if you decide not to unify, it would be good to at least do this one.

Regarding the mutation variables-I am not sure what that has to do with sql injection. You get SQL injection when you pass raw strings to sql query. I never do that in my apps. Also from the server side these should be equivalent. The graphql spec says:

A GraphQL query can be parameterized with variables, maximizing query reuse, and avoiding costly string building in clients at runtime.

I have it defined as ID in the schema, so using a parametrized query doesn't help me at all. It only makes usage of that query easier on the client.

Even if you passed an SQL command there, it would be escaped, because on the BE I call http://vincit.github.io/objection.js/#findbyid

capaj avatar Jun 05 '18 10:06 capaj