Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support file uploads #14

Closed
jaydenseric opened this issue Jan 29, 2018 · 22 comments
Closed

Support file uploads #14

jaydenseric opened this issue Jan 29, 2018 · 22 comments

Comments

@jaydenseric
Copy link

jaydenseric commented Jan 29, 2018

If there is interest I can raise a PR to add support for file uploads via GraphQL mutation variables according to the GraphQL multipart request spec pretty easily.

It would add less than 1kb to the client bundle and the API will be unchanged so it won't get in the way for users who don't need uploads.

The logic to add to the fetcher is only a few lines; you can see how it works in apollo-upload-client.

I looked to see if there is some sort of middleware system that could be used as an alternative to baking it in, but there isn't one?

@kenwheeler
Copy link
Contributor

That would be dope! Also, what would your ideal middleware setup look like?

@jaydenseric
Copy link
Author

jaydenseric commented Jan 30, 2018

I suggest per-request config, overridable by the user:

import { Client } from 'urql'
import getViewerToken from './helpers'

const client = new Client({
  request: (request, operation) => {
    // Can also be an async function if you want to await a few things to build the config.
    
    // `request` properties match https://developer.mozilla.org/en-US/docs/Web/API/Request#Properties
    // Populated with defaults the user can override. For example, a request may
    // have a JSON or FormData body, depending if there are uploads.

    // `operation` is the GraphQL operation object, in clase you want to
    // manipulate it or sniff something about it for overriding config.

    request.url = 'https://api.foo.com/graphql'

    // Token is retrieved at the time of each request.
    const token = getViewerToken()
    if (token)
      request.headers = {
        authorization: `Bearer ${token}`
      }
  }
})

The manipulated request object will be used immediately afterwards by fetch, so the user has final say over fetch options. This allows a user to do almost anything you can imagine, without having to learn a system of links or middleware.

By design it also prevents a lot of common noob mistakes such as setting credentials at client initialization instead of per-request.

Easy to implement for a really lean bundle; There is no need for deep merging of request config set at client initialization and in middleware.

@jaydenseric
Copy link
Author

I edited the example above a few times for polish.

@vladinator1000
Copy link

That would be awesome!

@kenwheeler kenwheeler added enhancement help wanted ⛏️ Extra attention is needed labels Feb 2, 2018
@kenwheeler
Copy link
Contributor

can this be accomplished with the fetchOptions/functional fetchOptions settings on the Client instance currently?

@pyrossh
Copy link

pyrossh commented Feb 4, 2018

Yes most probably.
If you see apollo-upload-client source,
https://github.com/jaydenseric/apollo-upload-client/blob/master/src/index.mjs#L35
It just modifies the fetchOptions if it is a file upload and changes the variables a bit.

@kenwheeler
Copy link
Contributor

Closing on the basis that this is achievable via fetchOptions

@morajabi
Copy link

morajabi commented Apr 1, 2018

@kenwheeler But we don't have access to query/mutation body in fetchOptions, do we? in order for it to be implemented, we need that data to attach a FormData to the request body.

@blorenz
Copy link

blorenz commented Jun 30, 2018

@kenwheeler I am in agreement with @morajabi. The current interface for fetchOptions is insufficient to support this. Do you have any additional info about this that we are missing?

@kitten
Copy link
Member

kitten commented Jun 30, 2018

@blorenz there’s potential to implement this as an “exchange” (our version of Apollo’s links) which is available on the “next” branch which will go out in the next release.

I’ll reopen this and let’s see what we can achieve 👍

@kitten kitten reopened this Jun 30, 2018
@kitten
Copy link
Member

kitten commented Feb 15, 2019

v1 is now released with exchanges! So if there's any takers for this issue, I'd be happy to offer guidance! There's unfortunately no docs yet, but they're in progress and coming soon

@stevenmusumeche
Copy link

@kitten I'm game if you can provide some guidance

@engleek
Copy link

engleek commented Feb 25, 2019

Hello all!

I needed to handle files, so I glued something together. It's really basic and needs cleaning up, but it's a start and it works for me.

import { print } from 'graphql';
import { filter, pipe, tap } from 'wonka';

const fileExchange = ({ forward }) => {
  return ops$ => {
    const preFlight$ = pipe(
      ops$,
      filter(operation => {
        if (operation.operationName !== 'mutation') {
          return true;
        }

        if (!operation.variables.file) {
          return true;
        }

        const { url } = operation.context;
        const { file } = operation.variables;

        const extraOptions =
          typeof operation.context.fetchOptions === 'function'
            ? operation.context.fetchOptions()
            : operation.context.fetchOptions || {};

        const fetchOptions = {
          method: 'POST',
          headers: {
            ...extraOptions.headers,
          },
        };

        fetchOptions.body = new FormData()

        fetchOptions.body.append(
          'operations',
          JSON.stringify({
            query: print(operation.query),
            variables: Object.assign({}, operation.variables, { file: null }),
          }),
        )

        fetchOptions.body.append(
          'map',
          JSON.stringify({
            0: ['variables.file'],
          })
        )

        fetchOptions.body.append(0, file, file.name)

        fetch(url, fetchOptions)
          .then(res => res.json())
          .then(json => console.log(json));

        return false;
      })
    );

    return forward(preFlight$);
  };
}

export default fileExchange;

@engleek
Copy link

engleek commented May 10, 2019

Is this still something people want? I could submit a PR with this code, but it really is thrown together.

@rafamel
Copy link

rafamel commented May 28, 2019

@engleek people want. people want. 😄

@jaydenseric
Copy link
Author

@engleek people should be able to use File, FileList, etc. instances anywhere in variables, not just under variables.file.

This is probably the best reference for how to prepare fetch options for regular and multipart requests:

https://github.com/jaydenseric/graphql-react/blob/1b1234de5de46b7a0029903a1446dcc061f37d09/src/universal/graphqlFetchOptions.mjs#L17

extract-files can be used to find and extract files without mutating the original variables object.

@NEO97online
Copy link

Yes, people want this. I would love to have it

@cowglow
Copy link

cowglow commented Jul 3, 2019

I'm working on a project that needs this

@rzane
Copy link

rzane commented Aug 1, 2019

I looked into creating an exchange to perform file uploads today. I was hoping to avoid completely reimplementing the fetchExchange, because that would be annoying.

I thought I'd be able to do this by creating an exchange and assigning a body in the fetchOptions. That works because the fetchOptions are merged in here.

Next, I realized that the request was being sent with content-type: application/json, because of this line. I though I could simply set content-type: multipart/form-data, but when the request got to my server, it was unable to parse it.

It's important that the content-type header is completely omitted. And I couldn't think of an easy way to do that.

Ideally, I think URQL could expose a function called createFetchExchange with the following signature:

type CreateFetchExchange = (fn: (op: Operation) => RequestInit) => Exchange;

That way, I could implement a custom fetcher without having to totally reinvent what it means to fetch.

@kitten kitten removed the help wanted ⛏️ Extra attention is needed label Aug 5, 2019
@JoviDeCroock
Copy link
Collaborator

Hey,

As a follow up I've made a guided exchange that will be added to the docs, here we take the fetchExchange and alter it to also function for file uploads.

PR: #370

@rzane
Copy link

rzane commented Aug 5, 2019

@JoviDeCroock, I totally understand that I can copy and paste an entire exchange and hack on it. My intent was to try to avoid the duplication of that code.

Someone is going to make a package that contains a wholesale copy of that fetch exchange, just to implement a tiny tweak. Then, when a change is made to URQL's fetch exchange, it'll be hard to remember to pull those changes back into the upload exchange. That's why I proposed #367.

@kitten
Copy link
Member

kitten commented Aug 22, 2019

Hiya, we've recently reduced the boilerplate around the fetchExchange and expose more helpers to create proper OperationResults which makes it easier to make your own fetchExchange.

Usually these solutions very much depend on your own setup and servers, so we chose not provide a one-fits-all solution to make a fetchExchange, but rather provide a guide on how to create a custom fetchExchange based on ours. https://github.com/FormidableLabs/urql/blob/master/docs/guides.md#adjusting-an-exchange

For now I'll close this issue. Happy to answer any questions about this on Spectrum though :) https://spectrum.chat/urql

@kitten kitten closed this as completed Aug 22, 2019
kitten pushed a commit that referenced this issue Feb 5, 2020
* (chore) - add replacePlugin -60B

* (chore) - avoid implicit return and needless var creation -10B
kitten pushed a commit that referenced this issue Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests