I want receive a empty string on mutation

[michelaquino]

Hi guys,

I have a follow mutation structure:

var userType = graphql.NewObject(graphql.ObjectConfig{
	Name: "User",
	Fields: graphql.Fields{
		"id": &graphql.Field{Type: graphql.String},
		"name": &graphql.Field{Type: graphql.String},
		"gender": &graphql.Field{Type: graphql.String},
	},
})

func mutation() *graphql.Object {
	return graphql.NewObject(graphql.ObjectConfig{
		Name: "mutation",
		Fields: graphql.Fields{
			"update": &graphql.Field{
				Type:        userType,
				Description: "Mutation to update a user",
				Args: graphql.FieldConfigArgument{
					"id": &graphql.ArgumentConfig{
						Type:        graphql.NewNonNull(graphql.String),
					},
					"name": &graphql.ArgumentConfig{
						Type:         graphql.String,
						DefaultValue: "",
					},
					"gender": &graphql.ArgumentConfig{
						Type:         graphql.String,
						DefaultValue: "",
					},
				},
				Resolve: SomeResolver,
			},
		},
	})
}

I want to make a query with empty string, like this: mutation { update(id:"abc", name: "New name", gender: "") {id}}

Is it possible?

The following function in file value.go does the validation:

// Returns true if a value is null, undefined, or NaN.
func isNullish(value interface{}) bool {
	if value, ok := value.(string); ok {
		return value == ""
	}
	if value, ok := value.(int); ok {
		return math.IsNaN(float64(value))
	}
	if value, ok := value.(float32); ok {
		return math.IsNaN(float64(value))
	}
	if value, ok := value.(float64); ok {
		return math.IsNaN(value)
	}
	return value == nil
}

Thanks

[nicolaiskogheim]

I'm not sure I understand your question. What should return the empty string?

[ghost]

I'm not sure, but I think @MichelAquino may be referring to the issue I see. Which is that isNullish treats the empty string ("") in go as being a null value. IOW, if you mark a field String! and return "", you'll get a error complaining that you returned a null in a non-nullable type.

I tried this in the reference JS implementation and it doesn't treat the empty string as null:

var { graphql, buildSchema } = require('graphql');
var schema = buildSchema("type Query {dontNullMeBro: String!}");
var root = { dontNullMeBro: () => '' };

graphql(schema, '{ dontNullMeBro }', root).then((response) => {
  console.log(response);
});

is fine, where as

var { graphql, buildSchema } = require('graphql');
var schema = buildSchema("type Query {dontNullMeBro: String!}");
var root = { dontNullMeBro: () => null };

graphql(schema, '{ dontNullMeBro }', root).then((response) => {
  console.log(response);
});

errors with Cannot return null for non-nullable field Query.dontNullMeBro

I don't see anything in the graphql spec suggesting that empty strings are treated as null.

Patches welcome?

[nicolaiskogheim]

Thank you for explaining. Yes, that seems strange.

[michelaquino]

Hi guys, sorry for delay. Yes, this is my problem.

When I create a query with empty string, I receive the response: has invalid value \"\".\nExpected type \"String\", found \"\".\n"

Is the issue #178 related?

[bbuck]

I suppose it could be beneficial to not use any zero values as null in GraphQL responses, but that kind of puts more burden on the resolvers. So unfortunately, in a language like Javascript, null can be assigned to anything. You can create a new object like:

var user = {
  username: null,
  email: null,
  createdAt: new Date()
};

Perfectly legal. However, the Go version might look like:

type User struct {
        Username  string    `json:"username"`
        Email     string    `json:"email"`
        CreatedAt time.Time `json:"created_at"`
}

func NewUser() User {
        return User {
                CreatedAt: time.Now(),
        }
}

So what do you think Username and Password should be treated as? Should they be considered "" as valid? It looks to me the intent should be considered the same. It's an even extra burden to say to the implementing user that they have to use pointer fields for structs in GraphQL, or even to say you need to handle zero values for yourself.

In reality, "" is nil, there is no way a zero length string carries more information than nil and it's perfectly plausible to convert null to "" in the consumer of the API.

All in all, at some point Go is not Javascript, so matching the JS implementation 1:1 is not always going to be an option. Sometimes an idiomatic Go approach should be taken here, and I tend to agree that zero values are nil in regards to responses.

[ghost]

@bbuck, I'll respectfully disagree.

I think you are assuming an implementation for a resolver which isn't required. As per type FieldResolveFn func(p ResolveParams) (interface{}, error), all resolutions return an interface{}, which is, of course, nullable.

The question isn't how do golang structs map to JS structs (the GraphQL spec is clear that it isn't attempting to bind directly to any language), it's how values returned by a golang resolver are converted to values over the transport.

In this case, The mapping from Go to GraphQL is perfectly clean. The empty string and null are distinguishable when they referred to by an interface{}.

FWIW, I think if you are mapping a golang struct with string-valued fields to a grapql type, those string fields should be non-null in the schema and then the implementation of the resolver is clean (the empty string is the empty string).

In JS, any variable (let, const, var, object property) can be null, but that's not the same as any type being null. JS is dynamically typed. null is neither a number, string or boolean type (String(null) === "null", "" !== null)

GraphQL makes a clear distinction between "" and null (String and String! are entirely different types). I don't see why the golang binding to it should simply ignore that.

[bbuck]

I'm not talking about implementations of resolvers. I'm talking about zero values in the Go programming language. In other words, a var name string is automatically allocated with strings zero value, "". The intent is not necessarily that this value should be a string so always treating it as such is wrong; however, per your use case (or the use case of the OP) it's obvious that they do seem to want to provide some significance to an empty string.

Unfortunately this creates a little bit of a larger problem. On the one hand, zero values are usually considered "unset." You can see this with the json package and how it handles something like this:

type User struct {
	Name string `json:",omitempty"`
	Age  int
}

func main() {
	u1 := User{Age: 21}
	u2 := User{Name: "Peter", Age: 22}

	uj1, _ := json.Marshal(&u1)
	uj2, _ := json.Marshal(&u2)

	fmt.Println(string(uj1))
	fmt.Println(string(uj2))
}

This is just illustrating that treating zero values as "empty" or "blank" (or nil even) is not unheard of or necessarily unexpected. Unfortunately, handling your use case will get pretty tricky. It's just as easy as "Don't make blank strings nil" or "Blank strings are valid values for String! fields." The reason is that you cannot express (in an Idiomatic way) without adding more information to the type and/or resolver. This intent can't be expressed cleanly with this particular language. Automatic resolvers could adopt tags of their own, like graphql: instead of using json: and have something like omitempty that turns blank string into nil values -- but then how does this work for non-automatic resolvers?

What I'm trying to say is that the best, and easiest way to handle this situation is in the client. Or use a non-zero length string with something like a zero-width Unicode space (ugly hacks, yes I know).

Perhaps not treating zero values as is the ideal solution. Although it breaks the use of non-pointer structs where zero-values should be failure points. My whole point boils down to no handling of zero values will serve everyone. If we wanted to do something like this we'd have to potentially encourage all consumers of the library to write their Go structs using only pointers. Which may be undesired. I'm less concerned about the implications of the consumer of the eventual GraphQL server, as that is entirely unrelated to this library and this codebase's purpose -- in other words, this library supports A Go-friendly GraphQL implementation for the server, not the client side.

[ghost]

@bbuck. I'm not especially interested in a debate here. I don't really have skin in the game. I've locally patched my vendored repo of this (extremely useful -- thank you) library to have the semantics I require.

It's a bummer to have to keep applying a patch, but I actually need to be able to return the empty string to clients and at the moment graphql-go literally prevents a service from returning the empty string as a value to a field which is typed either String or String!.

Having said that, I just don't think there's any sense in which your assertion that the empty string is somehow equivalent to null (or nil) is correct. No type theory, language that I know of, nor graphql itself take this view.

It's your library. If you think this behavior makes it somehow easier to create go graphql services, then by all means keep it.

[bbuck]

It's not mine, I just contributed to it.

I just don't think there's any sense in which your assertion that the empty string is somehow equivalent to null (or nil) is correct. No type theory, language that I know of, nor graphql itself take this view.

Aside from the discussion about Go and zero values (which is the actual crux of the argument) this point of mine is centered around data. Not types. A value of nil and a value of "" bear the same meaning in regards to the amount of data provided.

Aside from that though, I think I made a decent attempt at discussing why making this kind of change isn't quite as simple as you make it out to be. I get that you're coming from your singular use case at the moment. But you have to think about the fact that you're fundamentally changing behavior of a library that people other than yourself are already employing and using (as well as you) and so breaking changes like this require careful consideration. Next you should also consider the implications it has in regards to how values are normally treated in Go (as opposed to other languages) and attempt to find a solution that lets both parties settle on behavior that best fits their desires. Struct tags work partially, adding return values or extra return data from a resolver is possible, but cumbersome, and so forth.

I'm trying to initiate a discussion about how to best approach introducing something like this and that begins with first understanding why it behaves as it currently does and then how best to achieve the goal of how it can behave to behoove the rest of us.

In regards to having to patch it, that is pretty much the case you'll find with most large scale projects. Libraries by others for the general group of consumers are never going to fit your use case perfectly. Unless it's the exact same as the creators of the library. It seems perfectly within your rights and is expected for you to have to make modifications to reach your specific goals. It's obviously not as easy as just grabbing something from somewhere else, but it's how things go.

Best of luck to you.

[ghost]

@bbuck I'm honestly not trying to be a dick. Just take a step back from being defensive and consider the prospect that your premise is wrong.

If "" is the same amount of data as nil, is that different from 0? 0 is the go zero value for integers. false is the zero value for booleans.

graphl-go currently coerces any empty string to null. If that's correct, then why not coerce all values of 0 or false to null?

Whether it's prudent to make a breaking change to an API in use is without question important, but you have to be clear about whether there is a problem before you can have that discussion.

[vietthang]

@rafael-atticlabs I've just encountered same problem with empty string, and I totally agree with you about this. The empty string value should not be treated as nil. Can you share what did you patch to fix this problem? Can I just remove the "" check in "isNullish"?

[arv]

@vietthang Here is the change we did: https://github.com/attic-labs/graphql/commit/676b0c8da5c2147c81b05a31f6cded67186cf985

[vietthang]

@arv thank you

[bosgood]

To provide another perspective on this, I've got a similar issue but I came at it from the other angle. Our DB schema is also somewhat at odds with Go's string type, in that it occasionally has nullable VARCHAR fields, and the question arose about how best to get those through sqlx and then graphql-go.

My current solution doesn't require changing graphql-go at all, so maybe it'll be useful for someone else too.

// In your database/models:
type User struct {
	Name sql.NullString  // Or any other maybe/option/nullable type
}

// In your graphql schema, use this helper to define a nullable string field:
func resolveNullableString(p graphql.ResolveParams) (interface{}, error) {
	v := reflect.ValueOf(p.Source).Elem()
	if !v.IsValid() {
		return nil, fmt.Errorf("Invalid elem: %v", p.Source)
	}
	fieldName := convertFieldName(p.Info.FieldName)
	f := v.FieldByName(fieldName)
	if !f.IsValid() {
		return nil, fmt.Errorf("Missing field: %v", fieldName)
	}

	nullString := f.Interface().(sql.NullString)
	if nullString.Valid {
		return nullString.String, nil
	}

	return nil, nil
}

// You could also create other nullable types if necessary
var nullableStringField = &graphql.Field{
	Type:    graphql.String,
	Resolve: resolveNullableString,
}

// Then define your fields using that field:
fields := graphql.Fields{
	"name": nullableStringField,
}

This approach seems to be working for me for the time being, hope it helps someone else. Feedback appreciated.

[bigblind]

The "pop" library for working with databases solves this problem quite well in my opinion. They have a nulls package, with a Bool type, a String type, and so on for every primitive type in Go.

These types hold a value of their respective primitive type, and a Valid flag that is true when the value is not null.

[alimoli]

It does not affect only String, but also Float and Int where 0 and 0.0 are considered nullable values. This is a major issue that must be fixed immediately. I am wondering how this issue created on 30 Jan 2017 may still be opened. The solution provided by @bosgood is correct and it is something that I did many times also for issues related to date time. However, I think that this library cannot have a future if we don't fix definitely these data types (the basic concept of GraphQL).