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

Add directive @void to unify the definition of fields with no return value #1992

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

spawnia
Copy link
Collaborator

@spawnia spawnia commented Dec 1, 2021

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

See graphql/graphql-spec#906

Changes

  • Add directive @void to unify the definition of fields with no return value

Breaking changes

None, since the service provider is optional. When added by default, it will reserve the directive name @void and the type name Unit.

@spawnia spawnia added the enhancement A feature or improvement label Dec 1, 2021
stayallive
stayallive previously approved these changes Dec 6, 2021
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, on a technical basis!

I just don't care for the choice for Unit, it seems like a random name that easily clashes with userland (I now learned that this is a type theory thing and makes sense but only if you know the sauce). Why not use enum Void { VOID } or enum Empty { EMPTY } or enum Accepted { ACCEPTED } (close to a HTTP 202 response) or even enum Null { NULL } or something a little closer to what it is actually doing?

Regardless, this seems like a nice addition!

# Conflicts:
#	CHANGELOG.md
#	src/Console/IdeHelperCommand.php
@pyrou
Copy link
Collaborator

pyrou commented Feb 14, 2023

This PR becomes a bit old.
But for what it worth, here is what I use in my project :

scalar Void @scalar(class: "App\\GraphQL\\Scalars\\VoidType")

With following dummy implementation (a class simply named "Void" isn't allowed in PHP) :

class VoidType extends ScalarType
{
    public function serialize($value): mixed {
        return null;
    }

    public function parseValue($value): mixed {
        return null;
    }

    public function parseLiteral(Node $valueNode, ?array $variables = null): mixed {
        return null;
    }
}

By making Void a type rather than a directive, it becomes much more clear when defining the schema :

        type Query {
            foo: Void
        }

The approach proposed with @void directive ends with very unnatural definition in schema, like the one seen in unit test :

        type Query {
            foo: Int @void
        }

Is it void or Int ?

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

Successfully merging this pull request may close these issues.

3 participants