feat: support GraphQL Upload Spec #175
Conversation
There was a problem hiding this comment.
Thanks @lynxtaa!
Not sure if I should continue working on this PR (add tests, documentation) because the issue is closed
Issue re-opened.
Let's get this merged soon. Please add some tests and update the README (mention this feature, add an example).
src/createRequestBody.ts
Outdated
| const { clone, files } = extractFiles({ query, variables }, '', isExtractableFileEnhanced) | ||
|
|
||
| if (files.size > 0) { | ||
| const form = new FormData() |
There was a problem hiding this comment.
I'm using graphql-request in node env and tried using this implementation on the server in environment where FormData is not available. We might need to use something like https://www.npmjs.com/package/form-data for compatibility with Node env.
There was a problem hiding this comment.
I think Form-Data should be polyfilled just like fetch, but I'll add readable error if form-data is not defined and improve documentation
There was a problem hiding this comment.
Yeah, that's fair. Would be great if documentation would explain how to set this up for node.js.
There was a problem hiding this comment.
Am ok if we need another polyfill. We don't currently mention anything about the fetch polyfill in the docs. What needs to be said about the form-data polyfill?
There was a problem hiding this comment.
I've added documentation
I need help with tests, not sure how can I test multipart requests |
|
@lynxtaa best is probably some integration testing. Some generated files in a tmp dir during test and then upload to temp GraphQL server (express GraphQL maybe). Open to simpler if possible. Want confidence though. |
Added integration testing with Apollo Server. |
There was a problem hiding this comment.
Thanks for the test @lynxtaa. One more comment then I think we can merge.
First of all, thanks for this awesome library!
This PR adds support for GraphQL Multipart Request Spec and solves #132
Not sure if I should continue working on this PR (add tests, documentation) because the issue is closed