Schema stitching support

[yaacovCR]

Hi!

I have been working on graphql-tools-fork and although I am not sure myself that gateway forwarding of files to subschemas is the best idea, it certainly has been a requested idea (jaydenseric/graphql-upload#56, ardatan/graphql-tools#671), and I am hoping to support it within the fork.

One approach based on your comment (#79 (comment)) is to expand extract-files to just handle streams in addition to files.

This is a bit tricky for my use case, as what the fork does automatically is populate the delegated variables with the value of the variable from the first schema, i.e. a promise that resolves to an object with a createReadStream method, not a stream itself.

My thought is to spend a bit longer parsing the query based on the schema and to actually look for variables of type GraphQLUpload. Then I will await know that those promises if they successfully resolve will resolve appropriately to the correct streams, and I will await Promise.all() them. I think I can handle batching there, but I am not quite sure.

What do you think about this approach? It would require passing the schema manually on link creation, I believe, but otherwise I think it would be doable?

Do you think it belongs in a PR to this repository or a separate project?

[jaydenseric]

I'm sorry, I'm not across all the moving parts and I'm a bit rusty since I looked into it over a year and a half ago.

To get apollo-upload-client to support uploading files from a server, a new option needs to be added to tap into the new extract-files option isExtractableFile, so that it can be taught to also extract appropriate Node.js streams from variables as files.

I suspect though that schema stitching is not a valid use case for this. I wouldn't use Apollo Client on the server, I would just use fetch directly. You don't want all the bloat to do with cache, etc.

[yaacovCR]

I think you are right that I can just tap into extractFiles options. Seeing as they really should not be any promises in the variables map, I can just await all promises first and then replace them with the resolved values and then pass to extractFiles.

Schema stitching does not use a cache, it uses a bare Apollo link that acts like a fetch-like fetcher unless using subscriptions.

[jaydenseric]

Schema stitching does not use a cache, it uses a bare Apollo link that acts like a fetch-like fetcher unless using subscriptions.

Multiple kilobytes of needless complexity and dependencies? Just make a fetch request! Borrow most of the logic from here: https://github.com/jaydenseric/graphql-react/blob/v9.1.0/src/universal/graphqlFetchOptions.mjs

I may be missing something, you've spent more time thinking about it than I have :)

The only thing actionable here I think is for someone to PR a new isExtractableFile option to be passed down to extract-files, that way people can use apollo-upload-client on the server however they wish.

[yaacovCR]

An aside: you can use a fetcher instead of a link in schema stitching if you want.

I guess my issue is that in graphql-tools and the fork, we try to make schema stitching straightforward and provide default delegating resolvers.

The question I have is where to resolve the promises in the resolver arguments, I had thought that could be integrated into extractFiles as well but I see now that it maybe too much work to turn that into an asynchronous function.

I wonder if graphql-tools-fork should be responsible for resolving the promises within the variables map and the conversion of the resolved file object into the same format as expected in the upload client (essentially passing the result of createReadStream with name property set to filename) and then extractFiles can handle the further extraction of variables into the map.

[yaacovCR]

Related: node-fetch/node-fetch#707

[yaacovCR]

graphql-tools-fork v8.1.0 provides new exports to allow proxying remote file uploads for those who want to proxy all the things.

The new createServerHttpLink method should be used to set up a terminating link to a subschema; it will resolve all promises within the variables, and then, if appropriate, append the File or stream to the FormData. It includes an extended version of the FormData class that works even with the old version of node-fetch.

I based the createServerHttpLink on the latest version of apollo-link-http and apollo-client-upload. Would be happy to submit it as a PR, but it is intended for use on the server rather than the client. Perhaps it belongs in a separate project?

The GraphQLUpload scalar export from graphql-tools-fork just de-disables the serialize method as all args as serialized prior to proxying to the subschema.

Feedback/critiques/suggestions would be much appreciated. I am happy to submit PRs where appropriate, this is more POC.

[yasmin-ravenwolfe]

@yaacovCR Does proxying remote file uploads allow for stitching of remote schemas, in which one of those subschemas passes a File upload as a multipart/form content-type? I was taking a look at the source code in graphql-tools-fork, but couldn't get this working.

If you could send me some example documentation, I'll be happy to give feedback after implementing and contribute to any related issues. Thanks!

[yaacovCR]

Yes, that is exactly what it is supposed to support.

See test for now for example: https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/test/testUpload.ts

Unfortunately, the new features in graphql-tools-fork suffer from a paucity of documentation, see yaacovCR/graphql-tools-fork#21

I am looking for help with the repo in general as Apollo has essentially stopped supporting graphql-tools, docs would be a great place.

[yaacovCR]

Just noticed that the test defines an upload resolver in mergeSchemas, that resolver should at this point essentially be the default stitching resolver and not necessary, just had it in there when debugging, will update when I get a chance, see

https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/test/testUpload.ts#L110

You should be able to just take that out, simplifying the gateway further.

[yaacovCR]

@yazfaz if you have a code example of the new functionality not working, I would love to take a look

[yasmin-ravenwolfe]

@yaacovCR Thanks, that would be very helpful! I'll update my code with the new info you provided and open an issue on the graphql-tools-fork repo with the code.

I'll definitely submit a PR for documentation on this once it's working.

[yaacovCR]

See https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/test/testUpload.ts

Gateway code has been streamlined to just the basics even when proxying file uploads:

    const gatewaySchema = mergeSchemas({
      schemas: [subSchema],
    });

Magic happens within the createServerHttpLink in the subschema setup:

    const subSchema: SubschemaConfig = {
      schema: nonExecutableSchema,
      link: createServerHttpLink({
        uri: `http://localhost:${remotePort}`,
      }),
    };

You can also still define the subschema passed to mergeSchemas using makeRemoteExecutableSchema, but that adds another (unnecessary) layer of delegation, where you are proxying from the gateway to an intermediate local schema and only then to a remote schema:

    const subSchema: GraphQLSchema = makeRemoteExecutableSchema({
      schema: nonExecutableSchema,
      link: createServerHttpLink({
        uri: `http://localhost:${remotePort}`,
      }),
    });

[yaacovCR]

@jaydenseric Any thoughts as to whether you would want a PR? Basically, apollo-upload-client is described as:

A terminating Apollo Link for Apollo Client that allows FileList, File, Blob or ReactNativeFile instances within query or mutation variables and sends GraphQL multipart requests.

A PR would change that to:

A terminating Apollo Link that can be used for client-server connections (e.g. Apollo Client) or for server-server connections (e.g. schema delegation with graphql-tools-fork), that allows FileList, File, Blob, ReactNativeFile or Node streams within query or mutation variables and sends GraphQL multipart requests.

I think with this more generic support, a better name might be graphql-upload-link...

[yaacovCR]

@yazfaz have you had a chance to look into this?

[yasmin-ravenwolfe]

@yaacovCR Sorry for the delay- I've been off this project for the last few weeks, but will be implementing the graphql-tools-fork code this week. I'll be sure to update you and submit a documentation PR then, thanks!

[maryalfred25]

@yazfaz when this fork will be merged , i'm trying to using gateway with file upload .

[yaacovCR]

@jaydenseric I think #179 is nice and generic, the fact that it happens to support schema stitching is sort of a bonus, main idea is that it passes isExtractableFile to extractFiles, and allows customization of the FormData polyfill analogous to customization of fetch. Hopefully you will have a chance to review.

[jaydenseric]

Support for uploading files from a server environment has been published in v13.0.0 🚀