Allow manually specifying nil on input objects

[wesleyjellis]

Swift equivalent of Shopify/graphql_java_gen#28

This adds support for input objects so that you can manually set a property to nil and have that show up in the mutation sent to the server.

The weird part of this is that the argument to init are now double optional (eg: inputArg??). This is so that we have a way of distinguishing between a user supplying an argument and not when the value they supply could be nil.

My only concern here is that in order to manually specify nil in a constructor/init is to use .some(nil) (see the test). I feel like this is very non-obvious and could cause a bug in the future. I'm open to suggestions on how else to fix this.

[raulriera]

This needs .none instead of .some

[wesleyjellis]

I don't think so? I need a way to distinguish between an optional argument being set to it's default value (which is nil) and the user trying to tell me they want a property to be nil, hence the .some(nil)

Because it unwraps to nil, I can try to unwrap each of the optional arguments and then just use the value inside if it's there as the property's value

[raulriera]

Optional.none is an optional with nil, Optional.some(nil) is saying that the optional has a value, but when you unwrap it is nil. That would be making it a double optional like your description Optional(Optional(value))

var a: String? = .none
print(a)

var b: String?? = ""
print(b)

// output
nil
Optional(Optional(""))

[wesleyjellis]

Yes, I want something that you unwrap to nil. The problem is distinguishing between: foo(optinal: nil) and foo() which I can't do at a language level because within foo, optional will have a value of nil regardless of whether the caller provided the nil explicitly or not.

[ignacio-chiazzo]

This PR seems good to me.

Although adding none(nil) explicitly to check doesn't look nice I don't see any other way to do that.

Note: There is no way to check whether the input used is the default value or is the one that you entered explicitly.

func optionalTest(thing: Int? = nil) {
    if thing == nil {
        if expression { //there isn't any expression to check if the param was passed per param 
            print("default")
        } else {
            print("explicit")
        }
   }
}

One workaround to check whether a param was passed or not is to create two different functions, one that receives thing param and the other not. However, it's a mess when you have a lot of optional null params.

[ignacio-chiazzo]

We shouldn't accept assgining nil to a nested list object. If it's a list it should be [] or a list of objects but not nil. @wesleyjellis @dylanahsmith

[dylanahsmith]

We shouldn't accept assgining nil to a nested list object. If it's a list it should be [] or a list of objects but not nil.

Do you mean that we shouldn't let a null literal value to be sent for an optional list input field? Why? I didn't think there was any restriction like that in the GraphQL spec.

[ignacio-chiazzo]

Do you mean that we shouldn't let a null literal value to be sent for an optional list input field?.

Yes.

Why should I send a null literal value for an optional list? Don't see any case that it needs it.
If you want to delete all associations just send [].

@dylanahsmith
Also just started reading the thread where Facebook integrated this feature and saw your comments, you have more context than me.

[dylanahsmith]

Why should I send a null literal value for an optional list? Don't see any case that it needs it.
If you want to delete all associations just send [].

That's because you are thinking about mapping the data to active record associations, but we shouldn't think about our data model in that way. For instance, an association can't return a null value, but semantically a null value might be appropriate for the data model. For example, we may have a manually specified list of products in a collection, where the collection would start out empty or may become empty when products are deleted, but a collection with no list of manually specified products (i.e. null) might specify that the list of products are automatically chosen.

Conversely, I don't think we should actually encourage specifying null values as a way to delete data, since it isn't clear whether null will be interpreted by the server as deleting the field or whether the field will just be ignored. Unfortunately, there is nothing in the schema to indicate whether a field is optional or accepts an explicit null value, so it just wouldn't be clear from looking at the schema that the field is null settable. Which means that there are a lot of values that can't actually be set to null but which a null value could be explicitly sent for and would just be ignored. If we later decided to have null mean that the null value should mean the fields data should be deleted, that could cause existing clients to start deleting data unintentionally. So generally it is better to have a more explicit way to delete data, which would also be discoverable by looking at the schema.

I don't think it makes sense to impose arbitrary restrictions in this generic code generator. It should work with any GraphQL schema. However, if we want to add restrictions to the client for fields that shouldn't allow a null value, then it would probably be better to explicitly tell the generator what fields should allow a null value. That could be either as a set of fields that we know an explicit null value makes sense for or the generator could take a function that chooses based on the field type. I don't know if it is worth the trouble to add support for these arbitrary restrictions though.