GraphQLResultNormalizer doesn't match GraphQLResultReader's behavior when parsing nil value for optional fields

[youj]

Hi,

I've been running into an issue where my GraphQL response includes an optional field with a null value cannot be parsed by GraphQLResultReader from cache. The Record entry created by GraphQLResultNormalizer has a nil value, which cannot be parsed by GraphQLResultReader.

In the following example GraphQL response, avatarThumbnailUrl is an optional string field, which has null value for this node.

{
	"data": {
		"node": {
			"supporters": {
				"edges": [{
					"node": {
						"avatarThumbnailUrl": null
					}
				}]
			}
		}
	}
}

In GraphQLResultNormalizer.swift at line 47

func didParseNull() {
    valueStack.append(nil)
  }

Upon parsing a nil value, the result normalizer pushes a nil value on the valueStack, which is set to the record's value for cacheKey at line 35

  func didResolve(field: Field, info: GraphQLResolveInfo) {
    path.removeLast()
    
    let value = valueStack.removeLast()
    
    let dependentKey = [currentRecord.key, field.cacheKey].joined(separator: ".")
    dependentKeys.insert(dependentKey)
    
    currentRecord[field.cacheKey] = value
    
    if recordStack.isEmpty {
      records.merge(record: currentRecord)
    }
  }

When parsing from cache, GraphQLResultReader tries to parse the optional field from the record using the optional function in JSON.swift at line 48.

func optional(_ optionalValue: JSONValue?) throws -> JSONValue? {
  guard let value = optionalValue else {
    throw JSONDecodingError.missingValue
  }
  
  if value is NSNull { return nil }
  
  return value
}

This function throws missingValue error if the value is nil.

I believe this is a bug because this behavior causes these type of GraphQL responses to be not readable from cache.

I found by changing the nil value that was pushed onto the valueStack in GraphQLResultNormalizer to NSNull(), it fixes the problem.

func didParseNull() {
    valueStack.append(NSNull())
  }

[martijnwalraven]

@youj: Good catch! Thanks for going through the code and finding the root cause. Fixed on master, I'll release a new version as soon as the CI tests pass.