-
Notifications
You must be signed in to change notification settings - Fork 2k
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
First batch of edits to middleware docs #5404
First batch of edits to middleware docs #5404
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise looks great!
|
||
In this case, we recommend you swap out `apollo-server` for `apollo-server-express` (unless you are confident that you want to use a different Node.js framework). This change requires only a few lines and has a minimal effect on your server's existing behavior (`apollo-server` uses `apollo-server-express` under the hood). | ||
|
||
> We recommend Express because it's the most popular Node.js web framework, and it integrates well with many _other_ popular frameworks. It does have its limitations (for example, Express async support is not built around `Promise`s and `async` functions), but backward incompatible changes to the framework are rarer than in newer frameworks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/frameworks/libraries/
|
||
After you get up and running with the "batteries-included" `apollo-server` package, you might want to configure its HTTP behavior in ways that this package doesn't support. For example, you might want to run some middleware before processing GraphQL requests, or you might want to serve other endpoints from the same server. | ||
|
||
In this case, we recommend you swap out `apollo-server` for `apollo-server-express` (unless you are confident that you want to use a different Node.js framework). This change requires only a few lines and has a minimal effect on your server's existing behavior (`apollo-server` uses `apollo-server-express` under the hood). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have a section here that directly shows how to swap out (ie what lines of code to write)? @trevor-scheer was saying that it felt like that was lacking and this seems like the right spot. (Not a regression in this sub-PR, shouldn't block merge)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely! I'm going to leave out of this PR and add to next
``` | ||
$ npm install apollo-server-express express graphql | ||
```bash | ||
npm install apollo-server-express express graphql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't put the comment directly on it but yikes, the line const { url } = await server.listen();
should NOT be in the apollo-server-express example
Something else we need to include (in this PR or on mine) is that And specifically, the "swap" instructions should have you call |
* docs: refactor middleware and ApolloServer docs Now the "apollo server" reference page is just about ApolloServer (we lose docs for `gql` but that's not the biggest deal; we can add a reference page about other exports later if we want), and the middleware page provides sample code for every middleware and gives advice on which middleware to use. Fixes #5097. (More or less. We could definitely do more work on this front, but this is a good start.) * First batch of edits to middleware docs (#5404) * First batch of edits to middleware docs * Incorporate feedback from @glasser * Remaining edits to middleware docs (#5408) Co-authored-by: Stephen Barlow <stephen@apollographql.com>
Not done but think these edits are all ok standalone