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

Bring back composable middlewares in apollo-server 2 #1308

Open
green-pickle opened this Issue Jul 7, 2018 · 28 comments

Comments

@green-pickle
Copy link

green-pickle commented Jul 7, 2018

Hey, coming from this one.

I'd love to try apollo-server-koa 2 and would love to use composable middlewares, similar to how it is done in apollo-server-koa 1 now.

Unfortunately, now it's possible to use only ApolloServer and no middlewares are exported. While it can help beginners or reduce time to setup server, this way of things lacks the transparency and adds another layer of magic which is not always desired.

ApolloServer could be another way of doing things, without removing separate middlewares. Please bring back this middlewares, they are very convenient and easy to see what's going on.

@apollo-bot apollo-bot bot added the feature label Jul 7, 2018

@unhawkable

This comment has been minimized.

Copy link

unhawkable commented Jul 16, 2018

I've been stuck on this for half a day today, trying to mix my other middlewares in (e.g. jwt authorization) so that they go before graphql, but no success

@lcjnil

This comment has been minimized.

Copy link

lcjnil commented Jul 17, 2018

+1 for this, this is not so Simplicity for use.

@thomasdavis

This comment has been minimized.

Copy link

thomasdavis commented Jul 18, 2018

We use to mount Apollo on const serviceRouter = express.Router(); , this also no longer works in Apollo Server 2

@abernix

This comment has been minimized.

Copy link
Member

abernix commented Jul 19, 2018

Apollo Server 2.0 (e.g. the apollo-server package) aims to make the getting started experience with Apollo Server much easier by building the Express application and wiring up the various components automatically. Applications with more complex middleware configuration needs, particularly those utilizing custom middleware, should still be able to use the apollo-server-{integration} patterns, though the configuration is a bit less concise.

For those saying that it's not possible to compose middleware, have you attempted to use these patterns from the 2.0 migration guide?

@flisky

This comment has been minimized.

Copy link

flisky commented Jul 30, 2018

@abernix, Yes, I tried the applyMiddleware, but it doesn't work for me.

Basically, I was using Koa-Router to do graphql with an arbitrary multi-tenant prefix, but applyMiddleware does a static path check.

Any suggestion?

@abernix

This comment has been minimized.

Copy link
Member

abernix commented Jul 30, 2018

@flisky Are you passing path as an option to applyMiddleware?:

server.applyMiddleware({ app, path: '/your-own-path/graphql'});

For example, this sample server demonstrates running two separate Apollo Server 2 instances on the same express app, each as a separate middleware with different paths. I haven't tried using the Koa integration (i.e. apollo-server-koa), but the principle should be the same.

@flisky

This comment has been minimized.

Copy link

flisky commented Jul 30, 2018

Sorry, I cannot. The path would be any tenant name, which could be resolved dynamically, because tenant could be dynamic registered.

In apollo server 1.x, I would do

...
appRouter.post("/graphql", graphqlKoa(withContext(appSchema)));
...
router.use(
  "/:app(\\w*)",
  auth(),
  database(),
  appRouter.routes(),
  appRouter.allowedMethods(),
);

But I cann't find way to do this in apollo server 2

@vladshcherbin

This comment has been minimized.

Copy link
Contributor

vladshcherbin commented Aug 1, 2018

From the linked issue it was already clear that core team has no interest in this one and since v2 is out, the possibility is very close to zero. 😞

@alexFaunt

This comment has been minimized.

Copy link

alexFaunt commented Aug 2, 2018

I understand the aim of getting a GraphQL server running quickly with best practices, but this pattern is very restrictive and not transparent as to what is inside.

I would prefer an approach more similar to adopted for the apollo client, whereby apollo-boost was created which is a preset bundle of the other packages aimed at getting the client running ASAP, but preserved the integrity of individual packages.

@vladshcherbin

This comment has been minimized.

Copy link
Contributor

vladshcherbin commented Aug 2, 2018

@alexFaunt same idea was proposed here :)

@unhawkable

This comment has been minimized.

Copy link

unhawkable commented Aug 6, 2018

@abernix even if you put your middlewares before applyMiddleware to be run on the same path, where your own middlewares do something with koaContext e.g. add authenticated user object to it, the context object is reset when resolving graphql middlewares, meaning your changes to the context object are not available inside graphql resolvers - eventually making it's super-difficult to use context-related data

@sbrichardson

This comment has been minimized.

Copy link
Contributor

sbrichardson commented Aug 9, 2018

@abernix I'm curious to get your feedback on my current solution for bringing back ASv1 middleware style below. The main reason for this is to get access to graphqlExpress() to return a schema specific to the request. I removed typescript stuff just for readability here.

  1. Extended the ApolloServer with a custom applyMiddleware function().

This works, the main issue is having to copy graphqlExpress manually since
it's not exported any longer.

/* Have to import these extra libraries */
const { renderPlaygroundPage } = require('@apollographql/graphql-playground-html')
const accepts = require('accepts')
const { json } = require('body-parser')
const { ApolloServer, makeExecutableSchema } = require('apollo-server-express')

/* Don't think it's exported normally, get directly */
const { graphqlExpress } = require('apollo-server-express/dist/expressApollo')

class CustomApolloServer extends ApolloServer {
  applyMiddleware({ app, path }) {

    /* Adds project specific middleware inside, just to keep in one place */
    app.use(path, json(), auth, (req, res, next) => {

      /* Playground collapsed for readability */
      if (this.playgroundOptions && req.method === 'GET') { // ...
      
      /* Not necessary, but removing to ensure schema built on the request */
      const { schema, ...serverObj } = this

      /**
       * This is the main reason to extend, to access graphqlExpress(),
       * to be able to modify the schema based on the request
       * It binds to our new object, since the parent accesses the schema
       * from this.schema etc.
       */
      return graphqlExpress(
        super.createGraphQLServerOptions.bind({
          ...serverObj,
          graphqlPath: path,
          /* Retrieves a custom graphql schema based on request */
          schema: makeExecutableSchema(getCustomSchema(req)),
          context: {
            // ...other context
            request: req
          }
        })
      )(req, res, next)
    })
  }
}

const server = new CustomApolloServer({
  /* adding a default graphql schema initially */
  schema: makeExecutableSchema({ typeDefs, resolvers })
})

server.applyMiddleware({ app, path })

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

@sbrichardson sbrichardson referenced this issue Aug 9, 2018

Merged

Apollo Server 2 Koa integration #1282

1 of 1 task complete

Maledong added a commit to Maledong/egg-graphql that referenced this issue Aug 25, 2018

refactor (graphql.js): Upgrade to the v2 of apollo
We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the
middle-wares,
but now they have been either removed or hidden from the public. And
since v2
of apollo we've faced with a BREAKING CHANGE
(See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800
).
In order to be compatible with the older codes users are referring,
we have to do some tricks:

1) Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo'
instead
of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

2) 'apollo-server-module-graphiql' is removed, so in order to compatible
with
what it was, we have to add this in our package and reuse that. And then
we should
rewrite 'graphiqlKoa' function as the middle-ware copied from:
https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to
apollo's
version that is less than 2.x. And there're many ambigious problems
about this
design of v2 for apollo (See:
apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions
would be re-added
in the future (See:
apollographql/apollo-server#1282 (comment)).

Maledong added a commit to Maledong/egg-graphql that referenced this issue Aug 25, 2018

refactor (graphql.js): Upgrade to the v2 of apollo
We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the
middle-wares,
but now they have been either removed or hidden from the public. And
since v2
of apollo we've faced with a BREAKING CHANGE
(See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800
).
In order to be compatible with the older codes users are referring,
we have to do some tricks:

1) Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo'
instead
of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

2) 'apollo-server-module-graphiql' is removed, so in order to compatible
with
what it was, we have to add this in our package and reuse that. And then
we should
rewrite 'graphiqlKoa' function as the middle-ware copied from:
https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to
apollo's
version that is less than 2.x. And there're many ambigious problems
about this
design of v2 for apollo (See:
apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions
would be re-added
in the future (See:
apollographql/apollo-server#1282 (comment)).
@nfantone

This comment has been minimized.

Copy link

nfantone commented Aug 25, 2018

Related: #1282 (comment)

Maledong added a commit to Maledong/egg-graphql that referenced this issue Aug 25, 2018

refactor (graphql.js): Upgrade to the v2 of apollo
We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the
middle-wares,
but now they have been either removed or hidden from the public. And
since v2
of apollo we've faced with a BREAKING CHANGE
(See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800
).
In order to be compatible with the older codes users are referring,
we have to do some tricks:

1) Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo'
instead
of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

2) 'apollo-server-module-graphiql' is removed, so in order to compatible
with
what it was, we have to add this in our package and reuse that. And then
we should
rewrite 'graphiqlKoa' function as the middle-ware copied from:
https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to
apollo's
version that is less than 2.x. And there're many ambigious problems
about this
design of v2 for apollo (See:
apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions
would be re-added
in the future (See:
apollographql/apollo-server#1282 (comment)).
@Maledong

This comment has been minimized.

Copy link

Maledong commented Aug 25, 2018

@chentsulin:Not sure whether your team will expose the middle-wares in the next release or in the future?
And if yes, please have a link to this issue so as we all know about that :)

jtyjty99999 added a commit to eggjs/egg-graphql that referenced this issue Aug 26, 2018

refactor (graphql.js): Upgrade to the v2 of apollo (#18)
We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the
middle-wares,
but now they have been either removed or hidden from the public. And
since v2
of apollo we've faced with a BREAKING CHANGE
(See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800
).
In order to be compatible with the older codes users are referring,
we have to do some tricks:

1) Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo'
instead
of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

2) 'apollo-server-module-graphiql' is removed, so in order to compatible
with
what it was, we have to add this in our package and reuse that. And then
we should
rewrite 'graphiqlKoa' function as the middle-ware copied from:
https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to
apollo's
version that is less than 2.x. And there're many ambigious problems
about this
design of v2 for apollo (See:
apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions
would be re-added
in the future (See:
apollographql/apollo-server#1282 (comment)).
@vladshcherbin

This comment has been minimized.

Copy link
Contributor

vladshcherbin commented Sep 22, 2018

@evans hey, can you tell us (the users of apollo-server) if middleware way of handling things is planned for apollo-server v2?

Personally, I can't use v1 forever (graphql 14 and up already give you a warning) but I don't like the design of v2. If you don't have plans to bring back support / change the way v2 is doing things, we can create a separate package with separate middlewares and use it instead.

I don't want to pollute npm with such package if you have plans for this but if you don't, it would be great to have your confirmation before start. Thank you.

@hwillson hwillson added this to the Release Next milestone Sep 26, 2018

@Vincz

This comment has been minimized.

Copy link

Vincz commented Sep 30, 2018

I really need this too.
Maybe we could just split the Koa package ApolloServer code to expose one method on ApolloServer for each available middleware.
One for the "health check", one for upload, one for the graphql handler, one for the playground, one for cors and one for body parser.
This method would just return the middleware callback based on the provided options.
And the current "applyMiddleware", would just call this individuals methods.
This way, we could just use the middleware we want, the way we want.

@nfantone

This comment has been minimized.

Copy link

nfantone commented Oct 4, 2018

The thing is most of these are already available as separate packages or can be put together with little effort.

One for the "health check",

We could have an apollo-server-health-check exporting a Koa/Express compatible middleware. Or even being exported by apollo-server-koa itself.

one for upload,

https://github.com/jaydenseric/graphql-upload

one for the graphql handler,

I see this as the "primary" one, default export of apollo-server-koa on par with v1.

one for the playground,

This one should be entirely optional, I believe, and not even published/shipped to production.

one for cors,

https://www.npmjs.com/package/@koa/cors

and one for body parser.

https://www.npmjs.com/package/koa-bodyparser

@Vincz

This comment has been minimized.

Copy link

Vincz commented Oct 5, 2018

Yes, I agree that almost everything is available as a separate package.
But the community and packages are moving so fast, that it's nice to have just one package to maintain all the needed dependencies to easily do graphql at least for the combo graphql handler + playground + graphql upload.

@razor-x

This comment has been minimized.

Copy link

razor-x commented Oct 18, 2018

I am able to replicate the existing middleware behavior of v1 for Koa as follows

import koaBody from 'koa-bodyparser'
import { graphqlKoa } from 'apollo-server-koa/dist/koaApollo'
import { resolveGraphiQLString } from 'apollo-server-module-graphiql'

const graphiqlKoa = options => async ctx => {
  const query = ctx.request.query
  const body = await resolveGraphiQLString(query, options, ctx)
  ctx.set('content-type', 'text/html')
  ctx.body = body
}

// assuming koa-router

router.get('/graphql', graphqlKoa({ schema }))
router.post('/graphql', koaBody(), graphqlKoa({ schema }))
router.get('/graphiql', graphiqlKoa({ endpointURL: '/graphql' }))

There are few issues with this:

  1. It seems the graphqlKoa middleware is still in v2, just not documented or exported at the top-level anymore.
    This means that importing graphqlKoa like this is un-supported and un-documented,
    thus could break on any new release of apollo-server.
  2. It's uncertain if additional middleware is required to use graphqlKoa beyond bodyparser
    since the new the middleware stack is black-boxed in the new applyMiddleware function.
    Ideally the middleware required for graphqlKoa to work would be separated from the
    non-essential middleware like playground.
  3. It's unknown if apollo-server-module-graphiql is still supported or will be updated
    with new versions of GraphiQL.
    Many of our developers still prefer GraphiQL in some cases over playground,
    so we provide both.

@abernix abernix modified the milestones: Release Next, Release 2.2.0 Oct 24, 2018

bensaufley added a commit to bensaufley/typeorm-api-base that referenced this issue Nov 4, 2018

Updating Package, abandoning.
As mentioned in [this issue](apollographql/apollo-server#1308), Apollo Server 2.0 doesn't work with the way this repo had been built. This was just an experiment anyway (though with hope to use in real world applications), and considering my other hesitations about how Apollo works, this is just the deciding factor. I'm going to try a different approach.
@jonaskello

This comment has been minimized.

Copy link

jonaskello commented Nov 4, 2018

I also have a multi-tenant server with some specific authentication requirements. I was trying to upgrade to 2.0 today to do subscriptions but I'm stuck on this issue. The "simple made easy" clojure paradigm comes to mind. Middleware is simple and easy to compose and reason about. Trying to abstract the middleware away to make it "easy" does not make it "simple" to use IMO.

@jared-dykstra

This comment has been minimized.

Copy link

jared-dykstra commented Dec 4, 2018

I just wanted to confirm--Currently it's not possible to pass an express router instance to createServer? It would be fantastic if I could create a router, apply middleware to it, and then use the router to create the server.

Instead of this:

  const express = require('express')
  const app = express()
  const apiPath = '/api'

  apolloServer.applyMiddleware({ app, path: apiPath })

It would be fantastic if I could do this:

  const express = require('express')
  const app = express()
  const apiPath = '/api'
  const apiRouter  = express.Router()

  // middleware that is specific to this router
  apiRouter.use(function timeLog (req, res, next) {
    console.log('Time: ', Date.now())
    next()
  })

  // NOTE: cannot pass router to `apolloServer.applyMiddlware()`
  apolloServer.applyMiddleware({ router: apiRouter })

  app.use(apiPath, apiRouter)

See Related Stackoverflow Post: https://stackoverflow.com/a/53597429/5373104 Please correct me if I'm wrong or missing something

@green-pickle

This comment has been minimized.

Copy link
Author

green-pickle commented Dec 17, 2018

Almost half a year passed and there are no news about this one. It's pinned to version 2.2, but 2.3 was released and it's still not there.

Can we have any info from core team about status of this one?

cc @evans @abernix @peggyrayzis @martijnwalraven

@Maxpain177

This comment has been minimized.

Copy link

Maxpain177 commented Jan 22, 2019

+1

@daerion

This comment has been minimized.

Copy link

daerion commented Jan 30, 2019

Aye, +1 from me too - don't know why a completely non-standard way of doing things was implemented here.

@vladshcherbin

This comment has been minimized.

Copy link
Contributor

vladshcherbin commented Jan 30, 2019

For everyone watching - upvote and track this PR - #2244, maybe we'll finally get v1 methods instead of current horrible ones in v2.

@nfantone nfantone referenced this issue Jan 31, 2019

Closed

Export graphqlKoa #2244

3 of 4 tasks complete
@BrendanBall

This comment has been minimized.

Copy link

BrendanBall commented Feb 5, 2019

I need this to be able to use Apollo server 2. #1907. This is blocking me from using stable graphql 14.x which is problematic because most of the ecosystem has moved to stable graphql. Can we not make this higher priority? Only caring that newcomers have an easy time and not caring about existing users with proper production code with more complex requirements is really not cool

@nfantone

This comment has been minimized.

Copy link

nfantone commented Feb 5, 2019

@BrendanBall I understand your frustration. Maybe you'd like to express that sentiment over at #2244? At least there, some contributors are responding back and explaining things a bit and where the API might go from here.

@daerion

This comment has been minimized.

Copy link

daerion commented Feb 25, 2019

Since this was something we really needed for our product, we went ahead and wrote our own implementation of a koa middleware for an apollo server. It's heavily leaning on the implementation provided by apollo-server-koa and provides the exact same functionality and behavior for basic graphql queries and mutations.

It does not yet support subscriptions or file uploads, but other than that it should be a suitable replacement for those looking for a traditional middleware approach:

https://github.com/CrystallizeAPI/koa-middleware-apollo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.