Add isExtractableFile, appendFile, FormData options

[yaacovCR]

Closes #172. Simply adding customization options for apollo-upload-client greatly facilitates creation of a link that can support schema stitching.

See one implementation at:

https://github.com/yaacovCR/graphql-tools-fork/blob/with-apollo-upload-client/src/links/createServerHttpLink.ts

[yaacovCR]

Thoughts on this?

[jaydenseric]

Sorry to leave you hanging, I'm been absolutely flat out the last few days (1.5hrs sleep last night!). This approach is very close to what I had in mind. Could the Windows support related changes please be moved to a different PR? I'm not actually sure I'm keen to support Windows TBH; there can be quite extensive and nuanced things that need tweaking.

[yaacovCR]

Done.

[yaacovCR]

Except the part about making a separate PR, if you are not into merging, I will hold off for now on that. :)

[yaacovCR]

Hope you are sleeping better!

No real rush, just giving you a reminder. :)

[yaacovCR]

@jaydenseric anything I need to do to update this?

[jaydenseric]

I was working on documentation and changelog entries for it last night, and hope to finish it up sometime soon. I’ll take it from here :)

[yaacovCR]

Cool, thanks!

[jaydenseric]

Thanks for kicking this off 🙌

[jaydenseric]

A little more work needs to be done before this can be released - testing it out in with the example project revealed that the way FormData defaults to the global crashes Node.js during Next.js SSR, because the global is not defined.

[yaacovCR]

I guess it used to work because the link was never used server-side. I would think that is just incorrect usage, because in theory the link could be used server-side and so FormData should be required to be defined when using SSR via polyfill or argument.

Not sure, though!

[yaacovCR]

For backwards compatible, I guess FormData should be set to default at runtime rather than link creation, but that is not parallel to fetch, seems wrong.

[yaacovCR]

Actually, i guess whatever, as long as fetch and FormData treated the same

[jaydenseric]

It's a tricky one! Either way the solution will be a little opinionated. It's probably ok for FormData to be treated differently to fetch, since very few people are uploading files in SSR and everyone already polyfills fetch.

I'm not totally against requiring FormData to be either polyfilled or specified in config like fetch, but if we choose that solution we need figure out and document the best way to do it so that useless polyfills don't make their way into people's client bundles.

[yaacovCR]

I guess cross-fetch in the example is similarly required to be included, but not being used, I guess then the defaults for both should just be substituted at runtime, doubt significant increase in execution time/memory use.

[yaacovCR]

Do you want me to modify PR?

[yaacovCR]

I think runtime defaulting is least breaking change and that's the way I would go at this point.

[jaydenseric]

cross-fetch is done in such a way that node-fetch is eliminated from client bundles by the the sorts of bundlers/minifiers Next.js uses.

Do you want me to modify PR?

No, if you would like to work on a solution please do it as a new PR against master 👍

[jaydenseric]

To test it out with the examples project:

1. Download or clone the project: https://github.com/jaydenseric/apollo-upload-examples
2. Follow the setup instructions for API and app.
3. Run the API.
4. In the app directory, run npx install-from ~/local/path/to/apollo-upload-client/repo/dir. Run this anytime you test changes to the local apollo-upload-client files.
5. Start the app, and test in the browser.

[yaacovCR]

Not sure if this is windows thing:

C:\Users\Yaacov\Documents\github\apollo-upload-examples\app>npx install-from ....\apollo-upload-client
npx: installed 5 in 2.833s
(node:8816) UnhandledPromiseRejectionWarning: Error: spawn npm ENOENT
at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
at onErrorNT (internal/child_process.js:456:16)
at processTicksAndRejections (internal/process/task_queues.js:81:21)
(node:8816) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:8816) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.