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

Roadmap: add file upload feature to 1.0 #229

Closed
sandervanhooft opened this issue Nov 30, 2016 · 17 comments
Closed

Roadmap: add file upload feature to 1.0 #229

sandervanhooft opened this issue Nov 30, 2016 · 17 comments

Comments

@sandervanhooft
Copy link

sandervanhooft commented Nov 30, 2016

Hi all,

As suggested here:

Shouldn't the file upload features be considered part of the 1.0 Apollo Client release? I think the method proposed by @HriBB is a great starting point.

The proposed "method" is exemplified in these packages: graphql-server-express-upload and apollo-upload-network-interface.

Handling the data flow with the Apollo stack feels very natural. Most apps will require some form of file uploading on top of Apollo's data flow handling.

For my projects, it was the first and foremost reason to apply a custom redux implementation aside Apollo's. All other project requirements could be handled with Apollo - which is great! Because file uploading is flowing data as well, it however feels very unnatural to have to handle the upload "manually", outside Apollo. When integrating with the UI, things get complicated fast.

So I think there's a clear use case for file uploading with Apollo. Besides that, @HriBB has showed it can be done with the Apollo stack.

Therefore i.m.h.o., the Apollo stack 1.0 release cannot not be complete without file uploading functionality.

Kind regards,

Sander van Hooft

@stubailo
Copy link
Contributor

stubailo commented Dec 1, 2016

Can you elaborate on your use case? Is it that you want to fetch data about the new file after upload?

@sandervanhooft
Copy link
Author

sandervanhooft commented Dec 1, 2016

Good morning @stubailo ,

Ofcourse. I hope the following use case clarifies how much simpler Apollo could make file uploading.

User wants to add an offer to a market place using a form:

  1. User enters title, price, availability, description
  2. User adds pictures
  3. User saves the offer
  4. User sees offer detail page, including images

If using Apollo as primary transport, after some client-side validation multiple calls need to be made to the backend:

  • Image upload to REST endpoint, returns image urls
  • Graphql mutation with offer title, price, availability, description, image urls
  • Some way to roll back if one of the above fails

Due to the multi-request nature of this solution, it is over-complex, fragile and error-prone.

If using a REST endpoint as primary transport for uploads, and use Apollo for the other transports:

  • Single POST request to REST endpoint with formdata including all info and the images
  • Retrieve the details page info with Apollo & Update the Apollo store's offer list query

It seems inconsistent to bypass Apollo like this for such a common task in an app architected around Apollo. It also requires a specific REST endpoint for this task alone.

Preferably, the offer creation including images can be done with a single Apollo-roundtrip:

  • Graphql mutation with offer title, price, availability, description, image files.

This way, there's only a single request to validate and respond to. Plus a lot of code/complexity can be factored out from the client. In most cases the Apollo store will be updated automatically with the mutation results.

Both the front-end team and back-end team will be happy with this simplicity.

Kind regards,

Sander

@HriBB
Copy link
Contributor

HriBB commented Dec 1, 2016

I think that file upload should be handled using external modules/packages. Many apps might use CDN for storing files, and in that case, GraphQL server should have no awareness of file uploading whatsoever.

@sandervanhooft
Copy link
Author

@HriBB I agree that Apollo should be storage agnostic. It should focus on what it does best: simple data transport using Graphql.

My plea is to include file uploads in these transport features.

@sandervanhooft
Copy link
Author

Addendum: the storage specifics (CDN etc) can then be handled at either the client or the server, depending on the project requirements.

@vladinator1000
Copy link

Having file uploads would be lovely! I was just wondering how to upload form data with a picture through a mutation.

@HriBB
Copy link
Contributor

HriBB commented Dec 7, 2016

@savovs file upload through mutations is already possible. Take a look at https://github.com/HriBB/apollo-upload-network-interface and https://github.com/HriBB/graphql-server-express-upload and let us know what you think.

@sorenbs
Copy link

sorenbs commented Dec 9, 2016

@stubailo do you guys have a concrete plan for how this should work. We are about to implement mutation based file upload for https://graph.cool and want to align closely with Apollo.

@drnachio
Copy link

I have the same problem.

@helfer
Copy link
Contributor

helfer commented Dec 21, 2016

@sorenbs We haven't really had to deal with file upload, so we haven't thought about it too much. It looks like there is enough interest in this topic that we might pick it up soon, though.

@sorenbs
Copy link

sorenbs commented Jan 23, 2017

@helfer That's excellent news! Anything we can do to help move this forward? It seems like @HriBB has done an excellent job exploring a concrete implementation already.

@jferrettiboke
Copy link

The method proposed by @HriBB is great, however you are missing some features like Multer validation rules. In this example made by @HriBB, Multer is used one time like Express middleware before calling GraphQL.

IMO, is better use Multer inside each GraphQL resolve function. If you use this way, you will be able to setup different validation rules for each resource.

I am trying to figure out this, but I need some help. See this issue for more information.

@sorenbs
Copy link

sorenbs commented Feb 5, 2017

That's a great point @jferrettiboke Do you think this is something that it would make sense to tackle during the Apollo Contributor Week? https://dev-blog.apollodata.com/save-the-date-apollo-contributor-week-eef66d09f0e6#.pj0yj67qu

@jferrettiboke
Copy link

Hi @sorenbs.

Yes. I think so. There are different use cases where developers have to validate the files before upload them (extension and size) depending of the resource (user photos, user videos, etc).
If developers use only one endpoint which accepts any file, they could not write specific validation rules for each files as I said.

Anyone could help on the issue I mentioned to bring another approach to the table?

@helfer
Copy link
Contributor

helfer commented Apr 19, 2017

@thebigredgeek I think you should tune in here with the file upload solution you were working on!

@thebigredgeek
Copy link
Contributor

thebigredgeek commented Apr 19, 2017

Well, I am not working on it yet haha. I just have a spec for how to serialize and deserialize files without having to use special-case resolver params etc. Was waiting until we had a spec for the network interface we were discussing on slack. I figured we were going to bundle file upload handling into the new interface?

@helfer
Copy link
Contributor

helfer commented Jun 28, 2017

Closing this for now. We can continue the discussion on apollographql/apollo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants