Output object removes values instead of setting to null

[paulpdaniels]

I am writing a Scala implementation of the apollo-upload-server that follows the existing specification here. I ran into an issue with how the extract-files library processes the variables object when it contains a File object. The specification says that all file objects should be replaced with null, however, this line is deleting the field instead. Meaning that for an object like:

{
  "query": "mutation Upload($file: Upload!) { uploadFile(file: $file) }",
  "variables": { file: File }
}

The variables object gets converted into "variables: {}" during file extraction rather than "variable": { file: null }. Which should I be following? Or is the expected behavior just one or the other?

[jaydenseric]

I've been a little cheeky, and am not strictly following my own spec. I initially considered making the spec more complicated to allow this behavior, but in the end decided it was easier to simply say "replace all file instances with null".

Technically the null placeholder is only necessary when a file is an array item, so the indexes don't get messed up. object-path in apollo-upload-server has exactly the same outcome for repopulating a file as an object property whether the null placeholder is there or not.

This extract-files package simply uses delete on files instead of replacing them with null when extracting. This removes object properties but only deletes array index values, not the actual indexes. Later, when the tree gets converted to JSON in apollo-upload-client, JSON.stringify conveniently converts the empty array index values to null for us.

End-to-end in my implementations, everything works slightly more efficiently by not strictly following the spec for object property file values. Slightly less processing, and less sent up the wire.

Which should I be following?

Good question. Do you think I should:

1. Allow this in the spec.
2. Update either extract-files or apollo-upload-client to strictly conform to the spec.
3. Leave things how they are, and just explain to observant people the strangeness in my implementations.

[paulpdaniels]

(2) would be the easiest for me personally, since I would only need to run npm upgrade for apollo-upload-client. But I see the reasoning and am fine with making my parser more lenient if that would avoid potential breaks or bugs from being introduced. (1) is the next best option and might avoid weirdness for other people. I don't know how many others are taking the same route and trying to implement this spec from scratch, but at the very least it would save others time in the future.