-
Notifications
You must be signed in to change notification settings - Fork 234
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
File upload plugin #126
Comments
You should be able to access that in your resolver as |
Ciao Matteo, The problem is the request never hit the resolver. the error |
Well, I did it. |
@artecoop Nice one! Do you have a branch somewhere where I can look? I'm also digging into this issue :) |
@bmullan91 working on it. |
Youu sent is to the wrong bmullan
The person you want is "bmullan91" not me "bmullan"
…On Thu, Feb 20, 2020, 5:40 AM Art & Coop ***@***.***> wrote:
@bmullan <https://github.com/bmullan> working on it.
The code is now a bit of a mess, and for debugging purposes I've copied
files from grapqhl-upload package that needs to be imported (as
dependencies should work lmao)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#126>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM23J5AOQIK4KLXVRSPIG3RDZMZJANCNFSM4KXW7PDA>
.
|
Yeah sorry for that! |
@artecoop After a lot of head scratching and try and error... I got it working with this before registering the fastify.addContentTypeParser("multipart", (req, done) => {
req.isMultipart = true;
done();
});
fastify.addHook("preValidation", async function(request, reply) {
if (!request.req.isMultipart) {
return;
}
// processRequest from `graphql-upload`
request.body = await processRequest(request.req, reply.res, options);
}); Here's a codesandbox with the code https://codesandbox.io/s/fastify-gql-upload-eg-7gsz9 - note It fails to write the file on sandbox, not sure if that's possible on codesandbox. When I moved the code into a separate plugin it no longer worked - not sure why tbh 😕 |
@bmullan91 how could this work? graphql-upload's processRequest uses, for example, I'm prepping a repo with my code to share with all of you. |
@artecoop Im passing the raw req, and resp via: await processRequest(request.req, reply.res, options); |
@artecoop I've pushed up that sandbox into an actual repo: https://github.com/bmullan91/fastify-gql-upload-eg |
@bmullan91 this is beatiful. Please publish it as npm package. It's way better than my implementation that rewrite the body. |
@artecoop Will do, once I figure out why fastify doesn't like it when I put it inside its own plugin 😅 |
@bmullan91 check this out: |
@artecoop https://www.npmjs.com/package/fastify-gql-upload - still need to write tests etc, but this should unblock you 😄 |
Thank you, it works like a charm |
Made a fork of |
Brilliant work @PabloSzx !!! |
Could this be something that should be documented in the README, with a reference to @PabloSzx npm package? |
Should we support this natively? |
Apollo Server has out of the box support for File Uploads using graphql-upload, I think it would be nice to support it too, and it's not much code needed anyways, but there are some gotchas, like node version compatibility. |
IMHO, no. Keep going with the single responsibility principle, and let other packages, yours or made by other developers, plugs extra features. |
Might be a good idea to do a quick Pro/Con list (please complete it, if I miss something important) Pro
Neutral
Con
My subjective opinion, leave it out of core, but a note should be added to the |
My vote goes to not adding it in the core and leaving it as a plugin (and yes, I'm new to fastify-gql and my 2 cents probably isn't worth much). 😊 Thing is, how one handles uploading files with a GraphQL server can differ depending on needs and nailing down one way into the core is sub-optimal for everyone else not needing that method. The upload feature is then just bloat. Using plugins means a developer makes the conscious decision to add that feature, and at that point it isn't bloat. Scott |
closing, I think we have some sort of conclusion :). |
btw, the package was renamed to mercurius-upload following the rebranding |
@PabloSzx would you like to move it inside the org to give it an official blessing? |
Sure 😁 I added you (matteo.collina) as a mantainer in the npm package, but when I try to "Transfer ownership" of the GitHub repository, this error comes up:
|
Could you add me as an owner on npm as well? I am https://www.npmjs.com/~matteo.collina |
Done 👌 |
Great!!!! |
Maybe we can link the plugin from this project README as well? |
Hello again,
following #125 I'm trying to create a plugin to make possible to upload a file thru this amazing library, based on GraphQL multipart request spec.
I'm using fastify-plugin and fastify-multipart. I have created this stub:
With this code, I'm able to see the body in the console, like this:
Now, the endpoint return this error with HTTP 400
Must provide Source. Received: undefined
and the resolver is not executing.
My question is: how to pass the data from the response.body to the resolver's function?
I'm very sorry to post such open and broad question with little code, but I'm really not understanding how to complete the flow.
The text was updated successfully, but these errors were encountered: