jaydenseric / graphql-multipart-request-spec

A spec for GraphQL multipart form requests (file uploads).
1k stars 55 forks source link

Improve examples: Javascript objects are sorted, use file0 instead of 0 #14

Closed DHFW closed 5 years ago

DHFW commented 6 years ago

I had troubles understanding why I was not able to upload a file using the examples provided in this readme (Curl was working, but trying in Postman failed). Every time I used Postman (6.3.0) to upload a file I got this error: FilesBeforeMapUploadError: Misordered multipart fields; files should follow “map” (https://github.com/jaydenseric/graphql-multipart-request-spec).. The example had file indicated as 0. But, in Javascript the form-data object was sorted and caused this error. I took me (and other, see this error: postman issue. I think we could prevent others this search by updating the docs, hence this PR.

jaydenseric commented 6 years ago

Thank you for having the mindset to help others through a gotcha 👏

Really the issue is with Postman though, our examples are correct according to the spec. What if someone tries to make requests using file field names other than what is suggested in this PR (which is permissible according to the spec)? They will run into the Postman bug again.

People might mistakenly think that file field names need to begin with the word file when writing new implementations, which is more characters in a request than just using numbers.

in Javascript the form-data object was sorted and caused this error.

That is just an issue with the way Postman handles input. In JavaScript with FormData.append() the fields are appended in order, and that is how real client implementations do things: https://github.com/jaydenseric/graphql-react/blob/v2.0.1/src/graphql.mjs#L216

DHFW commented 6 years ago

@jaydenseric Thank you for your reaction! I can understand that you maybe wouldn't want to change the examples. But as I (and others, as well as Postman so it seems) have dealt with using an form-data object, the "bug" arises. You could consider mentioning in the readme to beware of this?

jaydenseric commented 5 years ago

A Postman fix has been merged: https://github.com/postmanlabs/postman-request/pull/29

This gotcha will be resolved once it's published, if it isn't already 🙏