graph-gophers / graphql-go

GraphQL server with a focus on ease of use
BSD 2-Clause "Simplified" License
4.64k stars 491 forks source link

Support treating Zero Values as Null #619

Closed bmhatfield closed 10 months ago

bmhatfield commented 1 year ago

This PR adds optional support for treating zero values as null. We primarily use this to allow us to seamlessly return Protobuf generated types from our resolvers, avoiding the need to implement schema-conforming translation types for output. This saves us a tremendous amount of boilerplate.

We have been running with this fork in production for quite some time (over 4 years!), which I have been shepherding and keeping up to date all this time (see here for our working branch).

If users do not specify AllowNullableZeroValues, the default is to preserve existing behavior.

pavelnikolov commented 1 year ago

I'm not convinced that this should be part of the library. Can this be a custom type? How does this exactly save you from tremendoues amount of boilerplate. I am also using this library with protobuf in every single resolver and the vast majority of my resourvers call some microservice(s) using gRPC. Can you give a simple example?

bmhatfield commented 1 year ago

Thank you for taking a look at this PR!

Here's a relatively simple example from our real code, that will hopefully illustrate the value of this change.

We have a proto message like:

// A minimal representation of a contact that belongs to a person or user
// record.
message Contact {
  // The person's given name (optional)
  string given_name = 1;

  // The person's family name (optional)
  string family_name = 2;

  // The person's email address.
  string email_address = 3;
}

Which of course generates as:

// A minimal representation of a contact that belongs to a person or user
// record.
type Contact struct {
    state         protoimpl.MessageState
    sizeCache     protoimpl.SizeCache
    unknownFields protoimpl.UnknownFields

    // The person's given name (optional)
    GivenName string `protobuf:"bytes,1,opt,name=given_name,json=givenName,proto3" json:"given_name,omitempty"`
    // The person's family name (optional)
    FamilyName string `protobuf:"bytes,2,opt,name=family_name,json=familyName,proto3" json:"family_name,omitempty"`
    // The person's email address.
    EmailAddress string `protobuf:"bytes,3,opt,name=email_address,json=emailAddress,proto3" json:"email_address,omitempty"`
}

The most accurate GraphQL representation of the intention of this type is:

type Contact {
    givenName: String
    familyName: String
    emailAddress: String!
}

However, this won't pass schema conformance:

string is not a pointer
  used by (*digits.Contact).GivenName

Without this change, to bridge between protobuf and graphql, we'd need to create an "output type" like:

type Contact struct {
    // The person's given name (optional)
    GivenName *string
    // The person's family name (optional)
    FamilyName *string
    // The person's email address.
    EmailAddress string
}

At the scale of our repo, that results in a significant amount of boilerplate.

Instead, we are able to return the generated protobuf struct directly.

As an inverse case, protoc-gen-go also always generates each field which is a ~struct type as a pointer to that type. If the field (ie, myfield: StructType!) is required, this also fails conformance.

This change also allows those structs to be returned directly, rather than needing to clone the type to remove the pointer.

I considered how one could implement this behavior with custom types instead, and an answer isn't jumping out at me. Apologies if you've got something straightforward in mind that we're missing.

Again, thanks for taking some time to look at this and apologies for not including clearer examples up-front.

pavelnikolov commented 11 months ago

What if you actually make these fields optional? For example:

// A minimal representation of a contact that belongs to a person or user
// record.
message Contact {
  // The person's given name (optional)
  optional string given_name = 1;

  // The person's family name (optional)
  optional string family_name = 2;

  // The person's email address.
  string email_address = 3;
}
bmhatfield commented 11 months ago

Thank you for this example. I can confirm that it generates structs with pointer fields which are compatible with the GraphQL schema. It appears that the optional keyword did not exist at the time we created this branch initially.

I will take a look at our code to see at if it makes sense for us to switch optional or if there's a reason to continue with this PR.

[edit]: learned why we did not use optional initially: it did not exist in proto3.