Apollo Server 2 Koa integration

[chentsulin]

Todo:

• fix tests

Implement it based on express one.

[abrenneke]

Whoops, should have checked for an existing MR before trying this myself.

Anyway, if there's anything here that you like, have at it: abrenneke@bb8aee9

Main differences:

• async/await instead of then/catch
• no hard dependency on koa-router (just uses a simple middleware wrapper)
• koa-bodyparser options have a concrete (inferred) type instead of Object

Otherwise, we did everything identically.

All tests are passing, though I didn't copy over apolloServerHttp.test.ts yet.

[chentsulin]

Hi @SneakyMax,
good to see your awesome code here. I noticed those items too when I wrote it.

1. async/await instead of then/catch

I agree that we can have better readability with async/await, but it didn't be used in apollo-server-koa v1, so I just follow it. - https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-koa/src/koaApollo.ts

2. no hard dependency on koa-router (just uses a simple middleware wrapper)

Either middlewareAtPath or koa-router are fine to me.

3. koa-bodyparser options have a concrete (inferred) type instead of Object

I open a PR here to export bodyParser.Options: DefinitelyTyped/DefinitelyTyped#27047

[vladshcherbin]

Is it possible to use it as a middleware, same as v1?

This package was so small, simple and elegant. Now it uses koa-router inside. 🤷‍♂️

[chentsulin]

@vladshcherbin Whether we use koa-router or not, it can be used as a middleware:

const server = new ApolloServer({ typeDefs, resolvers });

const app = new Koa();
server.applyMiddleware({ app });

app.listen({ port: 4000 }, () =>
  console.log(`🚀 Server ready at http://localhost:4000${server.graphqlPath}`),
);

[vladshcherbin]

@chentsulin why would you want another router inside if you already have one? I understand, it can be easier for newcomers to set up server or faster to start project, but what if I already have a router and want just middleware?

Koa is very minimal by its nature and this pollution with unnecessary stuff is weird. Is it possible to also export just middleware, same as v1?

[chentsulin]

@evans #1088

• Which approach should we use react-router, react-mount or SneakyMax's middlewareAtPath?
• Should we use (inferred) type for body parser or wait until [koa-bodyparser] export bodyParser.Options DefinitelyTyped/DefinitelyTyped#27047 be merged?

[chentsulin]

@vladshcherbin react-router just a implement detail now, and I am open minded to change it to a minimal middleware that handles pathes (/graphql, /.well-known/apollo/server-health) and keep tests pass at the same time.

Every server adapters (apollo-server-hapi, apollo-server-lamda, ...) are following express implemented interface.
https://www.apollographql.com/docs/apollo-server/v2/whats-new.html

If you just want a simple middleware, why not use v1 instead?
Though we can export graphqlKoa, but it share 99% the same code with v1 graphqlKoa.

[vladshcherbin]

@chentsulin I'd be happy to continue using simple v1 middleware, but I also want uploads out of the box. If it would be still possible to have two functions (one for gql and one for playground) which you could add to your routes, it would be super cool 😉

Something similar to how it's working now:

router
  .post('/graphql', koaBody(), apolloUploadKoa(), graphqlKoa(async ctx => ({
    schema,
    formatError,
    context: {
      user: await getUserFromRequest(ctx.cookies)
    }
  })))
  .get('/graphiql', graphiqlKoa({ endpointURL: '/graphql' }))

Easy to understand what's going on, what middlewares are used, no hidden magic inside.

[vladshcherbin]

So, what I'm proposing is to export bare middlewares (for file uploads, graphql, health check, playground) and combined apollo server with all this inside plus cors, body parser, router, etc.

Similar to how apollo-boost works.

[chentsulin]

@vladshcherbin This could be a separated proposal to Apollo Server 2, but not the scope of this PR.

IMO, if we accept this, it should apply to express and hapi too.

apollo-server/packages/apollo-server-express/src/index.ts

Lines 1 to 21 in 94d4d49

	export { GraphQLOptions, gql } from 'apollo-server-core';
	export {
	ApolloError,
	toApolloError,
	SyntaxError,
	ValidationError,
	AuthenticationError,
	ForbiddenError,
	UserInputError,
	} from 'apollo-server-core';
	export * from 'graphql-tools';
	export * from 'graphql-subscriptions';
	// ApolloServer integration.
	export {
	ApolloServer,
	registerServer,
	ServerRegistration,
	} from './ApolloServer';

apollo-server/packages/apollo-server-hapi/src/index.ts

Lines 1 to 21 in 94d4d49

	export { GraphQLOptions, gql } from 'apollo-server-core';
	export {
	ApolloError,
	toApolloError,
	SyntaxError,
	ValidationError,
	AuthenticationError,
	ForbiddenError,
	UserInputError,
	} from 'apollo-server-core';
	export * from 'graphql-tools';
	export * from 'graphql-subscriptions';
	// ApolloServer integration.
	export {
	ApolloServer,
	registerServer,
	ServerRegistration,
	} from './ApolloServer';

[evans]

@chentsulin Thank you so much for the PR! This is super awesome.

I really like the decisions made with @SneakyMax's code(Thank you so much). My review tends to lean in that direction.

There are a couple comments left from the express specific days that we should take out. I'd love to get this merged and released soon!

[evans]

we should also include graphql, since it's a peer dependency

[evans]

The principle of POST request was removed with persisted queries passed over a GET request, so we should remove these design principles

[evans]

The argument to graphQLServerOptions is passed to the context with some additional properties spread into it, so it might make sense to pass {ctx} or {req: ctx.req, res:ctx.res}}. Then the context function would either be ({ ctx }) => ... or ({ req, res }) => .... What do you think is more expected by Koa developers?

[chentsulin]

I'd like to see the whole ctx in the context, so ctx.request, ctx.session are accessible.

[evans]

Using the middlewareFromPath makes sense to me from SneakyMax's example sounds great, since we're not doing fine grain routing.

[evans]

It looks like async/await is much cleaner than using promises, so it sounds like a great addition.

[evans]

This note is express/connect specific, so we can take it out

[evans]

Let's infer the Options interface for now and leave a comment pointing at DefinitelyTyped/DefinitelyTyped#27047. Thank you so much for opening that PR @chentsulin!

[evans]

This was probably left over from the express integration. We should remove this, since applyMiddleware can be called multiple times with different paths.

[evans]

@vladshcherbin We definitely hear you on wanting different building blocks to put together. AS2's goal is to reduce the boilerplate necessary to build a fully functional GraphQL server. Reducing the effort to bring in features is definitely one of our goals.

You're definitely still able to use the same strategy of mixing many different middleware or components together with AS1, apollo-upload-server, graphql-apq, caching, and graphql-tools. For now, we've found this approach prone to errors that can fall outside of best practice and difficult to contribute to

[vladshcherbin]

@evans the reason why I'm asking it here is because this PR already has all this middlewares combined, the only thing to do is to split them into separate ones and export.

It also makes transition path easier for v1 users because if you want uploads for example, just add a new upload middleware in chain and you are good to go.

I'm not talking about removing combined ApolloServer, just give us a choice. 🙏

[nfantone]

Came here to second @vladshcherbin opinion on giving choice. The more I look at the new Apollo API, the more I like v1. IMHO, it looks very rigid and, more worryingly, condescending. Why the need to create a completely black, closed, non extendable box? Was it a pain point in the past having the freedom of choosing which middlewares your own server should have?

I get the idea of presenting a "great out-of-the-box experience" for newcomers. But I believe it would be wiser to not do that at the expense of letting down or constricting everyone else.

What are the new features that are included in ApolloServer? Caching? Uploading? Playground? Mocking? Let's turn each and everyone of those into distributable, configurable, composable Koa middlewares that can be mixed and matched individually. And build ApolloServer off of that composition. I'm offering myself a hand here if the team decides going down this road.

[evans]

@chentsulin and @SneakyMax Thank you so much for the PR! Really appreciate the effort ya'll put in to get this across the line. We'll release it in the next RC

[evans]

@vladshcherbin Definitely hear you on the worries. Curious if you have tried the transition, are there any pain points you ran into? Please open an issue and then we can discuss! If you're looking for uploads only, apollo-upload-server contains a koa middleware that you could import.

@nfantone Apologies that what I'm saying and the api comes off as condescending. AS2's goal is ease of use and reducing time to a production server, so tries to ease the setup burden. The constructor makes setting up subscriptions and other persistent connections easier along with adding graphql-extensions. If you see areas that we could improve on, I'd be happy to explore it in an issue. Are you specifically worried about something specific? Is it loss of functionality or does AS2 prevent you from accomplishing something?

[thomasdavis]

I've also commented on the issue raised about composable middlewares.

We run an express app where we rely on composing multiple Express routers and middlewares. We cannot change this architecture but would love to upgrade to AS2. The recommendation to simply stay on AS1 is concerning.

If Apollo cannot be composed or split up, you might as well just choose Express or something and bundle it by default. As in, I don't understand how to make the new Apollo Server anything but a stand alone server.

• How do I mount AS2 on a different route? (based on a nested router)
• How do I make AS2 use a different authentication system then other routes on my express app?

[nfantone]

@evans Many thanks for you answer! Let me expand a bit on what I tried to explain earlier.

AS2's goal is ease of use and reducing time to a production server, so tries to ease the setup burden.

Yes, we can see that. And in that sense, I believe you did an awesome job. This is a big step in that direction from AS1. No doubt.

Are you specifically worried about something specific? Is it loss of functionality or does AS2 prevent you from accomplishing something?

The problem I, personally, see with the approach you've taken is that it feels completely restrictive. While very good for adopting the framework, seems like many other present features have been hurt.

1. Loss of modularity. Most Node web frameworks, and koa in particular, are built around the idea of composing/chaining small middleware functions. AS2 declares several of them in one call with little in the way of configuration.
2. Lack of customization. What if my server doesn't need uploading capabilities? What if I want to switch to another implementation of it? How would I go about mounting some of those middlewares in other routes (as @thomasdavis points out)?
3. Dependency locking. New implementation depends not only on koa itself, but on a bunch of other popular middlewares, as well - which prevents developers from upgrading/downgrading their versions or using other modules of their choice without bloating their application.

These things, the way I tend to think about them at least, seem to go in detriment of the very philosophy behind frameworks like koa. And, ironically, somewhat against your own principles.

Simplicity: Keeping things simple [...]

There is nothing simple behind that server.applyMiddleware call.

The constructor makes setting up subscriptions and other persistent connections easier along with adding graphql-extensions.

But right there is just the thing: was that ever a pain point, to begin with? Setting up basic subscriptions or most other extensions is -luckily- pretty straightforward and documentation is already available. Unsurprisingly, these kind of things usually revolve around setting up a bunch of new middlewares in your app.

Finally, I'd like to go back again to something that was already mentioned here: this looks like a missed opportunity. Exporting individual modules while still providing the same ease of use experience doesn't seem like such a hard thing to accomplish and could have pleased both sides of the crowd.

[nfantone]

Also, on a related but distant note, I'd like to point out that many devs (most? I don't really have an statistic - perhaps you do) do not use Typescript to develop GraphQL servers. So the fact that Typescript-specific modules are being installed each time I download apollo-server-koa is rather underwhelming. Couldn't those be declared as devDependencies or optionalDependencies instead?

[vizo]

I came to this issue by searching on google how to disable playground in apollo-server-koa. But looks like i am forced to use it. At the moment playground have some serious issue graphql/graphql-playground#790 so i can't use it. I think this is perfect example why everything should be modular. Developers want to know what is happening behind. We are not (just) worpress users :D

Update

Sorry i was wrong about disabling playground, i just saw https://www.apollographql.com/docs/apollo-server/migration-two-dot.html#apollo-server-2.

[sbrichardson]

See this link on #1308

I ended up extending the ApolloServer class with a custom applyMiddleware function.
Curious for any feedback or potential issues.