Fix file upload related stuff to reflect reality #20
Conversation
…PI expects things now
newhouse
force-pushed
the
newhouse/fixUploads
branch
from
ebb8bc6 to
2c7bab6
Compare
|
|
||
| context('file is a Stream', function () { | ||
| def('variables', () => { | ||
| const file = fs.createReadStream(path.join(assetsDir, 'dummy.pdf')) |
There was a problem hiding this comment.
Does this provide the filename / mime? Will this fail asking for a filename / mime when a stream without that info is provided? I assume it would be specified just like the base64 one below?
There was a problem hiding this comment.
Great question, but no we are ok. As I wrote in our docs, form-data will be able to automatically suss out the filename and mime stuff from several "common" types of streams that it deals with:
Form-Data can recognize and fetch all the required information from common types of streams (fs.readStream, http.response and mikeal's request), for some other types of streams you'd need to provide "file"-related information manually
What happens is that it finds all that info in the "stream"-like thing's properties and it ends up in the multipart form head junk...and on the server side, GraphQL Upload will leverage it.
Our API supports form-data API's allowance for the manually passing/specifying of filename and mime stuff for these "non common" scenarios via the options param that I just referenced here, so users are good to go for all scenarios.
Basically, if form-data supports it (which is the only way we can support it), then we can also support it...but sometimes the User has to provide the options param depending on what stream-like thing they're going to use.
I've tried this against the API for reals and logged out a bunch of scenarios and it's all good.
The base64 case is obviously different b/c from the framework's point of view this is just your average JSON with some string value.
| const { | ||
| data: { | ||
| generateEtchSignURL, | ||
| }, | ||
| } = data | ||
|
|
||
| return { | ||
| statusCode, | ||
| url: generateEtchSignURL, | ||
| url: data?.data?.generateEtchSignURL, | ||
| errors, | ||
| } |
There was a problem hiding this comment.
Snuck in a tiny reformatting change here on some code I wrote earlier so that a potential error wouldn't be thrown. Doesn't affect rest of code. Hope that's alright @newhouse! 🙂
There was a problem hiding this comment.
Look at you with your optional chaining!!!
|
good go go @benogle ? |
Description of the change
Recent updates to the API around Uploads in GraphQL were not really reflected here. Let's get them up to date.
Mainly this was around how we used to "wrap" files in an envelope to mimic what Apollo GraphQL Upload was doing (and our backend liked/expected it). That has now changed, so no more wrapper.
However, this still required an alternate way for users to comply with "non-common" streams in our GraphQL operations by providing filename, mimetype and other options to the
form-data.appendcall...so I came up with a solution there as well.Also got the
base64upload stuff in line with the backend, and tested it out.Also got to remove a few dependencies...yay.
Type of change
Related issues
Fixes #1
Checklists
Development
Code review