Skip to content

Commit

Permalink
Use an express.Router rather than compose-middleware.
Browse files Browse the repository at this point in the history 
Aside from avoiding unnecessary dependencies, by using this Router, we avoid
the `apollo-server` base package from behaving different than it has in the
past.  Specifically, `apollo-server` uses `/` as the default path, but it
does so in a wild-card way which serves GraphQL requests on _any_ path.
That means that `/graphqlllll` and `/graphql` both also served the GraphQL
Playground interface, and also responded to GraphQL execution requests.

Since some may be leveraging that behavior, we should preserve it, if we
can.
  • Loading branch information
@abernix
abernix committed Aug 23, 2019
1 parent 368db67 commit c72277b
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 136 deletions.
45 changes: 0 additions & 45 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@
"@types/nock": "10.0.3",
"@types/node": "8.10.51",
"@types/node-fetch": "2.3.2",
"@types/parseurl": "1.3.1",
"@types/request": "2.48.2",
"@types/request-promise": "4.1.44",
"@types/test-listen": "1.1.0",
Expand Down
1 change: 0 additions & 1 deletion packages/apollo-server-express/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"apollo-server-core": "file:../apollo-server-core",
"apollo-server-types": "file:../apollo-server-types",
"body-parser": "^1.18.3",
"compose-middleware": "^5.0.1",
"cors": "^2.8.4",
"graphql-subscriptions": "^1.0.0",
"graphql-tools": "^4.0.0",
Expand Down
148 changes: 59 additions & 89 deletions packages/apollo-server-express/src/ApolloServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import {
import { ExecutionParams } from 'subscriptions-transport-ws';
import accepts from 'accepts';
import typeis from 'type-is';
import { compose } from 'compose-middleware';
import parseurl from 'parseurl';
import { graphqlExpress } from './expressApollo';

export { GraphQLOptions, GraphQLExtension } from 'apollo-server-core';
Expand Down Expand Up @@ -75,28 +73,6 @@ const fileUploadMiddleware = (
}
};

const middlewareFromPath = (
path: string,
middleware: express.RequestHandler,
) => (
req: express.Request,
res: express.Response,
next: express.NextFunction,
) => {
// While Express is quite capable of providing the `path`, `connect` doesn't
// provide `req.path` in the same way, even though it's available on the `req`
// as `req._parsedUrl`. That property is a cached representation of a
// previous parse done by the `parseurl` package, and by using that (popular)
// package here, we can still reap those cache benefits without directly
// accessing _parsedUrl ourselves, which could be risky.
const parsedUrl = parseurl(req);
if (parsedUrl && parsedUrl.pathname === path) {
return middleware(req, res, next);
} else {
return next();
}
};

export interface ExpressContext {
req: express.Request;
res: express.Response;
Expand Down Expand Up @@ -146,6 +122,8 @@ export class ApolloServer extends ApolloServerBase {
}: GetMiddlewareOptions = {}) {
if (!path) path = '/graphql';

const router = express.Router();

// Despite the fact that this `applyMiddleware` function is `async` in
// other integrations (e.g. Hapi), currently it is not for Express (@here).
// That should change in a future version, but that would be a breaking
Expand All @@ -160,33 +138,27 @@ export class ApolloServer extends ApolloServerBase {
// does. (And we'll take care to surface any errors via the `.catch`-able.)
const promiseWillStart = this.willStart();

const middleware: express.RequestHandler[] = [];

middleware.push(
middlewareFromPath(path, (_req, _res, next) => {
promiseWillStart.then(() => next()).catch(next);
}),
);
router.use(path, (_req, _res, next) => {
promiseWillStart.then(() => next()).catch(next);
});

if (!disableHealthCheck) {
middleware.push(
middlewareFromPath('/.well-known/apollo/server-health', (req, res) => {
// Response follows https://tools.ietf.org/html/draft-inadarei-api-health-check-01
res.type('application/health+json');

if (onHealthCheck) {
onHealthCheck(req)
.then(() => {
res.json({ status: 'pass' });
})
.catch(() => {
res.status(503).json({ status: 'fail' });
});
} else {
res.json({ status: 'pass' });
}
}),
);
router.use('/.well-known/apollo/server-health', (req, res) => {
// Response follows https://tools.ietf.org/html/draft-inadarei-api-health-check-01
res.type('application/health+json');

if (onHealthCheck) {
onHealthCheck(req)
.then(() => {
res.json({ status: 'pass' });
})
.catch(() => {
res.status(503).json({ status: 'fail' });
});
} else {
res.json({ status: 'pass' });
}
});
}

let uploadsMiddleware;
Expand All @@ -200,63 +172,61 @@ export class ApolloServer extends ApolloServerBase {
// Note that we don't just pass all of these handlers to a single app.use call
// for 'connect' compatibility.
if (cors === true) {
middleware.push(middlewareFromPath(path, corsMiddleware()));
router.use(path, corsMiddleware());
} else if (cors !== false) {
middleware.push(middlewareFromPath(path, corsMiddleware(cors)));
router.use(path, corsMiddleware(cors));
}

if (bodyParserConfig === true) {
middleware.push(middlewareFromPath(path, json()));
router.use(path, json());
} else if (bodyParserConfig !== false) {
middleware.push(middlewareFromPath(path, json(bodyParserConfig)));
router.use(path, json(bodyParserConfig));
}

if (uploadsMiddleware) {
middleware.push(middlewareFromPath(path, uploadsMiddleware));
router.use(path, uploadsMiddleware);
}

// Note: if you enable playground in production and expect to be able to see your
// schema, you'll need to manually specify `introspection: true` in the
// ApolloServer constructor; by default, the introspection query is only
// enabled in dev.
middleware.push(
middlewareFromPath(path, (req, res, next) => {
if (this.playgroundOptions && req.method === 'GET') {
// perform more expensive content-type check only if necessary
// XXX We could potentially move this logic into the GuiOptions lambda,
// but I don't think it needs any overriding
const accept = accepts(req);
const types = accept.types() as string[];
const prefersHTML =
types.find(
(x: string) => x === 'text/html' || x === 'application/json',
) === 'text/html';

if (prefersHTML) {
const playgroundRenderPageOptions: PlaygroundRenderPageOptions = {
endpoint: req.originalUrl,
subscriptionEndpoint: this.subscriptionsPath,
...this.playgroundOptions,
};
res.setHeader('Content-Type', 'text/html');
const playground = renderPlaygroundPage(
playgroundRenderPageOptions,
);
res.write(playground);
res.end();
return;
}
router.use(path, (req, res, next) => {
if (this.playgroundOptions && req.method === 'GET') {
// perform more expensive content-type check only if necessary
// XXX We could potentially move this logic into the GuiOptions lambda,
// but I don't think it needs any overriding
const accept = accepts(req);
const types = accept.types() as string[];
const prefersHTML =
types.find(
(x: string) => x === 'text/html' || x === 'application/json',
) === 'text/html';

if (prefersHTML) {
const playgroundRenderPageOptions: PlaygroundRenderPageOptions = {
endpoint: req.originalUrl,
subscriptionEndpoint: this.subscriptionsPath,
...this.playgroundOptions,
};
res.setHeader('Content-Type', 'text/html');
const playground = renderPlaygroundPage(
playgroundRenderPageOptions,
);
res.write(playground);
res.end();
return;
}
}

return graphqlExpress(() => this.createGraphQLServerOptions(req, res))(
req,
res,
next,
);
}),
);
return graphqlExpress(() => this.createGraphQLServerOptions(req, res))(
req,
res,
next,
);
});

return compose(middleware);
return router;
}
}

Expand Down

0 comments on commit c72277b

Please sign in to comment.