search

jaydenseric / graphql-multipart-request-spec

A spec for GraphQL multipart form requests (file uploads).
993 stars 54 forks source link

Conside using JsonPath for defining fiile field path. #61

Closed rossoha closed 2 years ago

rossoha commented 2 years ago

From my understanding field path for the case when we have an array of files: 

"SomeStructure":  {
"files" : ["file0", "file1"] 
}

will look like this: variables.SomeStructure.files.0 and variables.SomeStructure.files.1

Why not consider using JsonPath for field path definition? Cause at the moment we can have a situation when fields name that represents file can be literal or a number in resulting JSON with the same behavior as an array. Switching to JsonPath would slightly change path definition, but it would be parsable out of the box: variables.SomeStructure.files.[0] and variables.SomeStructure.files.[1]

Thank you @jaydenseric. Look forward to your response.

jaydenseric commented 2 years ago

What is “JsonPath” that you are referring to, is there a specification for it? Note that we already have this issue about adopting a proper path standard: https://github.com/jaydenseric/graphql-multipart-request-spec/issues/56 , which seems different to your proposal.

I'm not following your practical motivations for preferring .[0] vs .0 in the path string, what problems would it solve? Currently plenty of servers and clients with all sorts environments and languages using the current version of this spec are able to upload files held in arrays just fine.

If you want to put uploads in an object under string keys that are numbers, e.g. { "1": file, "2": file } I'm pretty sure it would work with the current spec, although I haven't personally done that before. The files are replaced with null in the variables JSON that is sent to the server, so the structure if it's an object with keys, or an array with items, is preserved. A sever implementation can see which type it is before populating the uploads back into their places.

jaydenseric commented 2 years ago

Closing as per the above comment.