Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turning Non-null to is_null #1505

Open
ManiMozaffar opened this issue May 19, 2023 · 3 comments
Open

Turning Non-null to is_null #1505

ManiMozaffar opened this issue May 19, 2023 · 3 comments
Labels

Comments

@ManiMozaffar
Copy link

Note: for support questions, please use stackoverflow. This repository's issues are reserved for feature requests and bug reports.
Hey :)
so I was reading docs, and then I remembered something:
simple is better than complex.

Why would we create a new class and object type because we don't want nullable values? isn't accepting a nullable value a property? then maybe we could have used is_null=True/False, and it gives us more control, because now we can have 4 possibility:

  1. is required, but accept null
  2. is not required, but accept null
  3. is required, but null isn't acceptable
  4. is not required, but null isn't acceptable
  • What is the current behavior?
    Using a new class/object, to adopt a behaviour that can be achieved as property

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via
    a github repo, https://repl.it or similar.

  • What is the expected behavior?
    Having is_null, beside required.

  • What is the motivation / use case for changing the behavior?
    This gives us more flexible, and less complex scalars.

  • Please tell us about your environment:

    • Version:
    • Platform:
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow)

@ManiMozaffar
Copy link
Author

I'd do the PR myself, but I wonder to see if that's something maintainers agree with me or not.

@tcleonard
Copy link
Collaborator

tcleonard commented Aug 15, 2023

Graphql has a weird relationship with nullable field.
The nuance of "a required nullable field" is not achievable as far as I know in the schema definition... indeed the "no value" is null and is valid for optional fields too.
Meaning that

type Mutation {
  doNothing(input: DoNothingInput!): DoNothingPayload
}

type DoNothingPayload {
  nullableInt: Int
}

input DoNothingInput {
  nullableInt: Int
}

that can be obtained with:

nullable_int = graphene.Int(required=False)

serves both purposes of being nullable and being optional.

Indeed you can do the following 3 behavior:

  • you provide an actual integer input:
mutation {
    doNothing(input: { nullableInt: 1 }) {
        nullableInt
    }
}

and in your resolver "nullable_int" in input is True (and input.nullable_int is 1 and input["nullable_int"] is 1 )

  • you can provide null input:
mutation {
    doNothing(input: { nullableInt: null }) {
        nullableInt
    }
}

and in your resolver "nullable_int" in input is True (and input.nullable_int is None and input["nullable_int"] is None )

  • you can provide no input at all:
mutation {
    doNothing(input: { }) {
        nullableInt
    }
}

and in your resolver "nullable_int" in input is False (and input.nullable_int is None and input["nullable_int"] actually fails)

So if you want to have a required nullable input, your only way to do it is to make the type required=False and add an assertion in your mutation

assert "nullable_int" in input

There is no way as far as I know to enforce that at schema level...

@tcleonard
Copy link
Collaborator

Note that this issue is an open conversation in the grapqh specs: graphql/graphql-spec#476

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

No branches or pull requests

2 participants