Apollo Server 2 Micro integration #1279
Conversation
hwillson
changed the title
|
Tests have been added, so I've removed the WIP. This should now be ready for review. Thanks! |
There was a problem hiding this comment.
@hwillson This is so incredible! I walked through the setup locally and it's great. I left some comments about the structuring of the content in the README and about graphqlHander vs createHandler. Also I'm going to defer to you on the cors, since I'm not sure where micro is most used. If it's ever used where another site might request from it, then we should probably include cors.
Thank you so much for putting this together!
|
|
||
| 4) After an `npm start`, access `http://localhost:3000` to run queries using | ||
| [`graphql-playground`](https://github.com/prismagraphql/graphql-playground), | ||
| or send GraphQL requests directly to `http://localhost:3000/graphql`. |
There was a problem hiding this comment.
For 2.0, we've been collocating these two endpoints and sending GraphQL Playground based on the accepts type sent with the request. If it's possible to do that with micro, that would be incredible.
There was a problem hiding this comment.
Just read the code and it looks like we do the colocation! I'm comfortable with this or to make the first link /graphql as well
There was a problem hiding this comment.
Great - I'll make them the same. Thanks!
|
|
||
| // Prepares and returns an async function that can be used by Micro to handle | ||
| // GraphQL requests. | ||
| public graphqlHandler({ |
There was a problem hiding this comment.
I think naming this createHandler would be better, since it lines up with lambda. Since Apollo Server is GraphQL specific, I'm not sure the graphql portion is necessary. Happy to differ to your judgement if the micro community expects something different
There was a problem hiding this comment.
Sounds good - I'll adjust, thanks!
| } | ||
|
|
||
| // Backwards compatibility with AS 1.x | ||
| export const microGraphql = graphqlMicro; |
There was a problem hiding this comment.
I don't think we need this, since we're on 2.0 and the other integrations don't export backwards compatible middleware/handlers
| ```js | ||
| const cors = require('micro-cors')(); | ||
| ... | ||
| module.exports = cors(apolloServer.graphqlHandler()); |
There was a problem hiding this comment.
Since micro is for microservices, I think best practice is to leave cors out by default, since microservices are supposed to be unaccessible by browsers. If you're experience is that they are called from another website in any regular usage pattern, then we should include cors by default.
There was a problem hiding this comment.
Let's leave CORS out for now. It's easy enough for end users to add in and we've documented a small example, so hopefully that's enough for now. We can always adjust this based on user feedback. Thanks!
| 1) Package installation. | ||
|
|
||
| ```sh | ||
| npm install --save micro apollo-server-micro |
There was a problem hiding this comment.
For now, we should stick with apollo-server-micro@rc. We'll replace it when the release comes out!
|
|
||
| ## Basic GraphQL Microservice | ||
|
|
||
| This example demonstrates how to setup a simple microservice using Micro, that |
There was a problem hiding this comment.
I think there needs to be a comma before using to make this grammatically correct
|
|
||
| ```js | ||
| ... | ||
| module.exports = cors(apolloServer.graphqlHandler({ path: '/data' })); |
There was a problem hiding this comment.
We should leave out the cors option here, since using the previous example. It might also be nice to link to the #basic-graphql-microservice example above when we mention it, in case forgetful readers(like me) didn't check the title 😊
There was a problem hiding this comment.
Ah I see, this tutorial is cumulative. I'd slightly lean towards standalone sections, since users can pull from the grab bag without something else sharp poking them. Though I'm happy to see it as a cumulative deal if you prefer. If it is building, I'm worried that people will get confused by the headers at the same level.
There was a problem hiding this comment.
Good point - I'll make each section standalone. Thanks!
|
Thanks for the review @evans! I believe I've covered everything (let me know if there's something I missed). |
|
@hwillson Thank you so much! |
Hi all - here's my pass at the Apollo Server 2 Micro integration (sorry for the delay @evans! 😳). This integration should be pretty much feature complete, based on the functionality offered by
apollo-server-express. A few comments:ApolloServer.applyMiddlewareapproach doesn't quite fit. Micro doesn't have middleware capabilities, so passing a Micro instance intoApolloServer, to make sure additional functionality is included during the request/response chain, won't work (well, we could make it work, but it would require a bit of questionable hacking). For this reason, instead of passing a Micro instance intoApolloServer, I'm building anasyncfunction inApolloServer(that includes all of the needed Apollo Server functionality) that can then be used in the calling application as the default exported Micro module. TheREADMEshows examples of how this works, but essentially the Micro handler function is generated usingApolloServer.graphqlHandler(). This is similar in concept to how theapollo-server-lambdaintegration works, but it's usingApolloServer.createHandler()instead. I prefer the more specificgraphqlHandlernaming, but I'm happy to change this is you'd prefer to see these lined up.apollo-server-micropackage though, definitely let me know.Associated with #1088.
Thanks!