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

Able to return None when you are not supposed to?? #229

Open
dcreekp opened this issue Oct 7, 2024 · 1 comment
Open

Able to return None when you are not supposed to?? #229

dcreekp opened this issue Oct 7, 2024 · 1 comment
Assignees

Comments

@dcreekp
Copy link

dcreekp commented Oct 7, 2024

Reporting issues with GraphQL-core 3

I came across an issue that may be due to Graphene but the problem surfaces here.

I was writing a mutation which would raise an Error or return:

{"data": {"someMutationThatReturnsNone": None}

instead of something like:

{"data": {"someMutationThatWillAlwaysBeTrueIfNoErrorRaised": {"success": True}}}

I was happy that this worked:

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = CustomNullTypeScalar

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

until I realised that this would also work:

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = graphene.Date(required=True)

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

currently, because the line of code linked above is before the check if is_leaf_type(return_type), so long as Output = someScalarOrEnumType the mutation will return {"data": {"someMutationThatReturnsNone": None}.
Which I don't think is right.

Would appreciate your thoughts.

@dcreekp
Copy link
Author

dcreekp commented Oct 7, 2024

Ok I just realised that this :

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = graphene.NonNull(graphene.Date)

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

errors as expected. Because the if is_non_null_type(return_type): will catch it first.

So maybe it should be enforced that Output class attributes are wrapped by graphene.NonNull in the Graphene library here.

But I guess that would mean that I will not be able to do what I want: create a CustomNullTypeScalar when I want a mutation to return None. There is nothing in the specs that I can find that prohibits returning something like: {"data": {"someMutationThatReturnsNone": None}.

Again would be interested in your thoughts @Cito 🙏

@Cito Cito self-assigned this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants