Extension to bring back file upload support

[jasonkuhrt]

Perceived Problem

Loss of file upload support #400

Ideas / Proposed Solution(s)

• need an extension system (tbd)

[jasonkuhrt]

Can someone please share the requirements for file uploads at runtime? I've never done it myself.

[ThallesP]

Can someone please share the requirements for file uploads at runtime? I've never done it myself.

hi, wdym by requirements? dependencies?

[jasonkuhrt]

Hey, I mean what does the implementation of file uploads need to do. Knowing details about that will help drive the extension design, because we'll know what kinds of knobs and levers need to be exposed.

[onionhammer]

For one thing, you have to change the type of request that's made from normal JSON data to multipart/form-data, you need to replace files in the payload with a reference to the file, etc, which means recursively searching the body for files.

It's not that hard, just fiddly to get everything 100%. See this code for reference:

https://github.com/lynxtaa/awesome-graphql-client/blob/a3ecec2fdd3b72b005268a24b2364dd647dc6efd/packages/awesome-graphql-client/src/AwesomeGraphQLClient.ts#L83

Written by @lynxtaa

which uses https://www.npmjs.com/package/extract-files

Spec: https://github.com/jaydenseric/graphql-multipart-request-spec

[jasonkuhrt]

Thanks, I'll check those links out when the time comes.

you need to replace files in the payload

what would "payload" be and what would "files in payload" mean? Does recursively searching the body mean the graphql document selection set?

[onionhammer]

By payload, I mean the object being POSTed;

i.e. https://github.com/jaydenseric/graphql-multipart-request-spec?tab=readme-ov-file#file-list

You can see the variables get swapped out for nulls, then a map is appended to the form data with the variables that were present in that list, but the shape of variables likely isn't known, so you have to search it for File objects.

[ThallesP]

Thanks, I'll check those links out when the time comes.

you need to replace files in the payload

what would "payload" be and what would "files in payload" mean? Does recursively searching the body mean the graphql document selection set?

Yep, also please accept a Readable stream from node or the web api, loading the entire file in memory using a Buffer should be optional and not the only way.

[jasonkuhrt]

I am getting close to reviewing that the new extension system has everything needed to support file uploads, and if not, make it so. I want this to happen give the demand from community so unless it would force major hacks into the lib I expect this supported.

As for an actual extension for file uploads, that would be a different issue and I would welcome community help with that. I do think some first party extensions make sense for popular topics like this. But it won't be my immediate focus as I work to get the foundation done.

[ThallesP]

I am getting close to reviewing that the new extension system has everything needed to support file uploads, and if not, make it so. I want this to happen give the demand from community so unless it would force major hacks into the lib I expect this supported.

As for an actual extension for file uploads, that would be a different issue and I would welcome community help with that. I do think some first party extensions make sense for popular topics like this. But it won't be my immediate focus as I work to get the foundation done.

are there any docs for extensions?

[jasonkuhrt]

@ThallesP Not yet, just tests, and minimal at that. You may be interested in #922.

[jasonkuhrt]

Things I see needed here:

1. Custom Scalar Upload. This will be used for typing input fields. Its TS type should be various forms of file blobs... browser/node... etc. (let's look for something modern and cross platform?)

2. The client needs to send null for the file variables to the server. Each file needs to become part of the multipart form request sent by client. This is where the runtime work/change happens.

3. Currently Graffle does not create GraphQL documents with variables. This will need to happen fist, in order to generate spec compliant requests.

For an extension to pull this off, it will need to alter the exchange hook implementation when files are being uploaded. This is not currently possible. It is not something Anyware currently supports. Perhaps it could though. Knowing when to alter could debased on sniffing the runtime types of inputs or on using the runtime schema map (more reliable).

Raw support could be added added. Only difference is that it would not have access to structured document to compare to the runtime schema map.

Overall I see a viable path here.

[onionhammer]

You won't be able to tell its TS type at runtime, this is how the other library handles it:

https://github.com/lynxtaa/awesome-graphql-client/blob/a3ecec2fdd3b72b005268a24b2364dd647dc6efd/packages/awesome-graphql-client/src/util/isFileUpload.ts

[lynxtaa]

how the other library handles it

For modern environments it seems like an overkill, I'm planning to make it simpler in my lib:

const isFileUpload = value => (typeof Blob !== 'undefined' && value instanceof Blob) 

And I suggest you the same since NodeJS 18 supports Blob natively and expects it to be passed to FormData

[jasonkuhrt]

You won't be able to tell its TS type at runtime, this is how the other library handles it:

Thanks, yeah saw that and the predicate helper. Actually, with Graffle, we can, because we have a runtime schema map (that is how custom scalars are transformed through their respective codecs). However, raw mode will not, and I'm inclined toward making it work across object and raw modes, even though using the schema map is the safest option. I may use both approaches, thus just making the raw mode a bit less safe which is pretty in line with its trade offs in the first place.

[jasonkuhrt]

Ok, I took a first stab at sketching the code and reviewing the referenced libraries. It turns out that our current extension system is powerful enough, if I got this right. Looks pretty simple actually!

const UploadExtension = Graffle
	.createExtension({ name: 'Upload' })
	.anyware(({ pack }) => {
		if (!isUsingUploadScalar(pack.input.variables)) return pack()

		// TODO we can probably get file upload working for in-memory schemas too :)
		if (pack.input.transport !== 'http') throw new Error('Must use http transport for uploads.')

		const { exchange } = await pack()

		const request = new Request(exchange.input.request, {
			body: createUploadBody({
				query: input.query,
				variables: input.variables
			}),
			headers: {
				'content-type': 'multipart/form-data'
			}
		})

		return exchange({
			...exchange.input,
			request,
		})
	})

/** @see https://github.com/lynxtaa/awesome-graphql-client/blob/a3ecec2fdd3b72b005268a24b2364dd647dc6efd/packages/awesome-graphql-client/src/AwesomeGraphQLClient.ts#L79 */
const createUploadBody = () => '...'

Can anyone point out any major flaws (to the extent you can make sense of the code :))?

[lynxtaa]

@jasonkuhrt Not a major flaw but the Content-Type header must include a multipart boundary. The easiest way to solve it is to not set the Content-Type header at all, it'll be correctly inferred by fetch

[jasonkuhrt]

@lynxtaa, thanks, that sounds good, however the cloned request will have had the content type header explicitly set, so I wonder in that case what happens, can try it sometime later.

[jasonkuhrt]

You can find implementation and tests here https://github.com/jasonkuhrt/graphql-request/pull/937/files#diff-5900de164f1c79c9265931d2e60cb70651486646ea5ebddf46429dfbc72c10dc. I will likely merge this PR this weekend. Any feedback welcome. The extension is small but some adjustments to the extension system to support its needs added to the size of the diff.