I'd like to reopen the discussion on @enjoylife's suggested simplification of the spec #50 . I believe their suggestion can greatly simplify this specification and ease the implementation for typesafe frameworks while still achieving the original goals for JS GraphQL libraries.

Just for clarity, this is a rough draft of how I imagine the new version of the spec could look building off of what @enjoylife has defined:

1. Requests are multipart/form-data
2. There is exactly 1 non-file form field. This contains the GraphQL request. The GraphQL server has a Scalar backed by a String that references the name of the related file form field.
   1. It doesn't need to be named operations anymore but it might be better to reserve this name for further extension
   2. It doesn't need to be before the file form fields but it might be best to enforce this for performance
   3. We might not need to specify the name of our scalar but Upload seems good.
3. Every other form field should be a file with a unique name which will be referenced by the Upload scalar.

ex:

curl http://localhost:4000/graphql \
  -F gql='{ "query": "mutation { upload(files: [\"file_id\"]) }", "variables": null }' \
  -F [email protected]

I've implemented a prototype of this in my own Scala server and I have a very rough implementation of this for Apollo:

Prototype Apollo Attachments Gist

Regarding the comments in #11 (describing how the map field is necessary for performance), this is an implementation detail of the server and something we can solve for Apollo. If our Apollo plugin finds an Upload(file_id) we grab a promise from our shared Uploads object which we will resolve once we parse our file or reject after we finish parsing the entire request. This lets us execute our GraphQL request as soon as we find it in our form fields.

This is a trace from running my gist:

curl http://localhost:4000/graphql \
  -F gql='{ "query": "mutation { upload(files: [\"file1\", \"file2\", \"file1\"]) }", "variables": null }' \
  -F file1=@Untitled \
  -F file2=@API\ Wizard.mp4

You can see that we've achieved the same async waterfall where our GraphQL request execution starts immediately.

The first thing that comes to mind (although it's a pretty exotic) is that the current spec allows files to be used anywhere in the GraphQL operations objects, not just in variables

Yes, each file is always referenced by its uid so your server can choose to arrange its json however it desires without any issues.

An added benefit of this proposal over the current spec is the ability to define file references outside of variables. Right now you're required to always have a "variables" section to reference via your map form-field. It's not possible to send something like:

curl http://localhost:4000/graphql \
  -F gql='{ "query": "mutation { upload(files: [\"file_id\", \"file_id\"]) }", "variables": null }' \
  -F [email protected]

With your proposal that doesn't include a map of where files are used in the operations, it's not clear to me how one file variable used as an argument in multiple mutations in the same request could be implemented on the server. Is that something you have considered?

This doesn't really change between the current spec and this proposal. You're always looking up in your context for the file based on its uid. There's no reason you can't repeatedly query the same file based on its uid.

ex:

curl http://localhost:4000/graphql \
  -F gql='{ "query": "mutation { a: upload(files: [\"file_id\"]) b: upload(files: [\"file_id\"]) }", "variables": null }' \
  -F [email protected]

Performance wise the map allows the server to cheaply see up front how many files there are, and where they are expected to be used in the operation without having to parse the GraphQL query looking for certain upload scalars in an AST, etc. For example, the map makes implementing the maxFiles setting in graphql-upload trivial.

Although this is true of the new spec change, we'll always be parsing GraphQL requests in GraphQL servers, it's a matter of leveraging the server libraries to facilitate this. This is something that could maybe be handled by an Apollo validationRule or definitely by an Apollo plugin. We're writing a spec for GraphQL, we should be using the tools our GraphQL servers provide to us.

Even if we're writing an implementation of this spec for a framework that gives 0 options to validate our GraphQL request, the current JS spec implementation has already defined code that would catch maxFiles as they were streaming through via Busboy: https://github.com/jaydenseric/graphql-upload/blob/2ee7685bd990260ee0981378496a8a5b90347fff/public/processRequest.js#L67

The point of this spec is to create a standard for interoperability between all GraphQL clients and servers, regardless of the languages or ecosystems

Exactly, this spec appears to be designed in order to run as a JS Server middleware. There is a good amount of indirection, implementation specific solutions, and dependencies on the implementing language/framework. This all creates more work for server implementers.

I did an audit of the various server implementations and all of the ones I looked at either depend on:

• Their language being dynamic
• Their language having a top Object type and casting (ignoring type safety)
• Or they basically implement the proposed spec change internally

There doesn't seem to be a good way to add this specification to a typesafe language/framework without it devolving into the proposed spec change.

For Example async-graphql in Rust

https://github.com/async-graphql/async-graphql/blob/f62843cbd34ef9bf28f70f8df07d4f61f8038e0a/src/request.rs#L115

*variable = Value::String(format!("#__graphql_file__:{}", self.uploads.len() - 1));

we can see that internally, after the map is parsed, we replace the null inside the variables definition with a uid to reference the specific file.

https://github.com/async-graphql/async-graphql/blob/f62843cbd34ef9bf28f70f8df07d4f61f8038e0a/src/types/upload.rs#L99

/// Get the upload value.
pub fn value(&self, ctx: &Context<'_>) -> std::io::Result<UploadValue> {
   ctx.query_env.uploads[self.0].try_clone()
}

When we get the value out of the Scalar, we pull the actual file stream out of the context via that same uid.

For Example caliban in Scala

https://github.com/ghostdogpr/caliban/blob/660beeae538768d817a73cb4535e0e3bd1a8cb82/adapters/play/src/main/scala/caliban/uploads/Upload.scala#L89

// If we are out of values then we are at the end of the path, so we need to replace this current node
// with a string node containing the file name
StringValue(name)

We're setting our variable value to the filename in order to pull it out of the context later.

For Example sangria in Scala

(This isn't actually in the library but this gist describes how to implement it).

https://gist.github.com/dashared/474dc77beb67e00ed9da82ec653a6b05#file-graphqlaction-scala-L54

(GraphQLRequest(gql = gqlData, upload = Upload(mfd.file(mappedFile._1)), request = request))

we store our uploaded file separately from our GraphQL request.

https://gist.github.com/dashared/474dc77beb67e00ed9da82ec653a6b05#file-controller-scala-L68

userContext = SangriaContext(upload, maybeUser),

we pass our file through via the context.

https://gist.github.com/dashared/474dc77beb67e00ed9da82ec653a6b05#file-exampleschema-scala-L15

val maybeEggFile = sangriaContext.ctx.maybeUpload.file.map(file => Files.readAllBytes(file.ref))

Inside of our resolver we lookup the file in the context to actually use it.

All of these examples have implemented @enjoylife's proposal under the covers in order to preserve some form of type safety.

Note:

We can use these libraries as a guide to show us how to implement supporting both the current version of the spec and the proposed change in the same server with plenty of code sharing.

For Reference graphql-upload in JavaScript

This JavaScript implementation depends on the language being dynamic so that we can overwrite our variables with an Upload instance.

https://github.com/jaydenseric/graphql-upload/blob/60f428bafd85b93bc36524d1893aa39501c50da1/public/processRequest.js#L232

operationsPath.set(path, map.get(fieldName));

We assign the Upload instance to the specified location in our GraphQL Json Request

https://github.com/jaydenseric/graphql-upload/blob/60f428bafd85b93bc36524d1893aa39501c50da1/public/GraphQLUpload.js#L81

if (value instanceof Upload) return value.promise;

When parsing our Scalar value, we check and cast to make sure we found an Upload instance.

For Reference graphql-java-servlet in Java

This Java implementation depends on the top level Object type so that we can check and cast our variables on the fly.

https://github.com/graphql-java-kickstart/graphql-java-servlet/blob/eb4dfdb5c0198adc1b4d4466c3b4ea4a77def5d1/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLMultipartInvocationInputParser.java#L138

objectPaths.forEach(objectPath -> VariableMapper.mapVariable(objectPath, variables, part));

We set each http.Part in our variable map

https://github.com/graphql-java-kickstart/graphql-java-servlet/blob/eb4dfdb5c0198adc1b4d4466c3b4ea4a77def5d1/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/apollo/ApolloScalars.java#L28

if (input instanceof Part) {
  return (Part) input;

When parsing our Scalar, we check and cast to make sure we found a http.Part instance.

This spec and the graphql-upload JS server-side implementation are not tied in any way to Apollo, or a "heavy js graphql abstraction"

I can't speak for @enjoylife but I don't believe the proposed changes to this spec are implying the code for graphql-upload is heavy. graphql-upload is quite elegant in its implementation. In fact, for my Apollo prototype I borrowed heavily from graphql-upload. The heavy parts are that:

• We have multiple implementation specific details baked into the specification
• Using null as a placeholder is really another server implementation detail, it doesn't make sense from the client perspective
• There is a lot of indirection in the variables in order to support implementing GraphQL Upload libraries inside of JS middleware

In Summary

"The point of this spec is to create a standard for interoperability between all GraphQL clients and servers, regardless of the languages or ecosystems" and the current iteration of this specification constrains non-dynamic languages in order to be written inside of a JS Server Middleware. Evolving this specification will better fit the growing GraphQL ecosystem and make this specification future proof so that everybody can benefit from the work you've done here.