Differentiate between explicit and implicit nil fields

[fruitcoder]

I have mutation that takes a EventCreateInput where every field is optional. Setting different fields and the combination thereof provides the graphql server with the information to run the mutation. But the problem is that providing null for certain fields causes the server to behave differently from not including the field at all.

Example:

public struct EventCreateInput: GraphQLMapConvertible {
  public var graphQLMap: GraphQLMap

  public init(clientMutationId: String? = nil, type: String? = nil, consumedBy: GraphQLID? = nil, content: GraphQLID? = nil, deviceIds: [String?]? = nil, firstInteraction: String? = nil, lastInteraction: String? = nil, progress: Double? = nil, reaction: GraphQLID? = nil, shares: [ShareCreateInput?]? = nil) {
    graphQLMap = ["clientMutationId": clientMutationId, "type": type, "consumedBy": consumedBy, "content": content, "deviceIds": deviceIds, "firstInteraction": firstInteraction, "lastInteraction": lastInteraction, "progress": progress, "reaction": reaction, "shares": shares]
  }
}

The events are interactions with an item. Multiple events are sent for a single item. When providing an id for reaction the server registers the reaction with that item. To remove the reaction from that item I need to give this input to the mutation:

[
   "content":"contentID", 
   "reaction": null
]

But this is not possible since we cannot differentiate between a parameter being explicitly set to nil and one being left out when creating EventCreateInput so it takes the default value of nil.

[martijnwalraven]

As mentioned on Slack, we used to generate initializers for all permutations of optional variables, so we could differentiate between not providing a value for a variable and providing nil. Unfortunately, this led to combinatorial explosion with too many optional variables (as seen in issues like apollographql/apollo-tooling#35), so I removed that workaround.

I don't see a way to get both strict typing and the ability to pass in NSNull(). So the cleanest solution is probably to expose an initializer that takes an untyped GraphQLMap for cases like these. What do you think? So you see any alternatives?

[fruitcoder]

I think exposing an initializer with an untyped GraphQLMap sounds like a valid solution. As you said, you cannot support type safety and NSNull in the same initializer unless you introduce a new type wrapper. I used something like this in a previous project but it might be too cumbersome to not deal with the "real types" and it's also harder to read:

enum TriValue<T> {
  case value(v: T)
  case none
  case null
  
  var resolved: Any? {
    switch self {
    case let .value(v):
      return v
    case .none:
      return nil
    case .null:
      return NSNull()
    }
  }
}
func test(string: TriValue<String>, bool: TriValue<Bool>) {
}

[martijnwalraven]

That's clever! But without language support for automatic wrapping (like you get for optionals), I agree it's too cumbersome.

I was thinking we could also expose a subscript or method to set null after construction:

var event = EventCreateInput(content: "contentId")
event["reaction"] = NSNull()
// or maybe something like:
event.nullify("reaction")

[fruitcoder]

This looks awesome! I like event.nullify("reaction"). I just realized I can already do event.graphQLMap["reaction"] = NSNull() so your solution is only convenience right?

[martijnwalraven]

Yes, that should work. I also like nullify, and we could add it as an extension on GraphQLMapConvertible if we make graphQLMap { get set }.

[fruitcoder]

Would this be enough?

public protocol GraphQLMapConvertible: JSONEncodable {
  var graphQLMap: GraphQLMap { get set }
}

extension GraphQLMapConvertible {
  mutating func nullify(key: String) {
    self[key] = NSNull()
  }
  
  subscript (key: String) -> Any? {
    get {
      return graphQLMap[key]
    }
    set {
      if newValue == nil {
        graphQLMap.removeValue(forKey: key)
      } else {
        graphQLMap[key] = newValue
      }
    }
  }
}

[martijnwalraven]

I think you'd want the type of the subscript to be JSONEncodable?, and I'm not sure we need the nil check to explicitly remove the value (shouldn't that happen by default when you assign nil to the map?), but otherwise it looks good!

[fruitcoder]

Yes and yes ;)

[martijnwalraven]

@fruitcoder: Going to close this issue, because I just merged what seems like a very clean solution! The generated input objects now have properties of nested optional types when the object fields are optional. That means we can differentiate between .none and .some(.none). The default value is still nil, so .none, which translates to the field not being present in the resulting map. But you can set any optional property to .some(.none), to indicate an explicit null.