Skip to content

Commit

Permalink
Fix Lambda context function typing
Browse files Browse the repository at this point in the history
In AS3 we made apollo-server-lambda inherit from apollo-server-express,
but didn't realize that it also inherited the more precise TS typings on
its constructor that meant that its `context` function would take
Express-specific parameters (rather than the default unconstrained
parameter).

This change makes it easier for any ApolloServer subclass to declare its
context function typing, and updates apollo-server-express and
apollo-server-lambda to do so. The other integrations are left with the
vaguer defaults.

Tests didn't catch this before because they didn't pass a `context`
function *directly* to the Lambda ApolloServer constructor, just via a
function with a looser signature.

Fixes #5480.
  • Loading branch information
glasser committed Jul 15, 2021
1 parent f1506c7 commit 79ca410
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 26 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ The version headers in this history reflect the versions of Apollo Server itself

## vNEXT

- `apollo-server-lambda`: Fix TypeScript types for `context` function. (In 3.0.0, the TS types for the `context` function were accidentally inherited from `apollo-server-express` instead of using the correct Lambda-specific types). [PR #XXXX](https://github.com/apollographql/apollo-server/pull/XXXX)

## v3.0.0

### BREAKING CHANGES
Expand Down
21 changes: 15 additions & 6 deletions packages/apollo-server-core/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,26 +109,29 @@ class UnreachableCaseError extends Error {
super(`Unreachable case: ${val}`);
}
}
export class ApolloServerBase {
export class ApolloServerBase<
// The type of the argument to the `context` function for this integration.
ContextFunctionParams = any,
> {
private logger: Logger;
public graphqlPath: string = '/graphql';
public requestOptions: Partial<GraphQLServerOptions<any>> =
Object.create(null);

private context?: Context | ContextFunction;
private context?: Context | ContextFunction<ContextFunctionParams>;
private apolloConfig: ApolloConfig;
protected plugins: ApolloServerPlugin[] = [];

private parseOptions: ParseOptions;
private config: Config;
private config: Config<ContextFunctionParams>;
private state: ServerState;
private toDispose = new Set<() => Promise<void>>();
private toDisposeLast = new Set<() => Promise<void>>();
private experimental_approximateDocumentStoreMiB: Config['experimental_approximateDocumentStoreMiB'];
private landingPage: LandingPage | null = null;

// The constructor should be universal across all environments. All environment specific behavior should be set by adding or overriding methods
constructor(config: Config) {
constructor(config: Config<ContextFunctionParams>) {
if (!config) throw new Error('ApolloServer requires options.');
this.config = config;
const {
Expand Down Expand Up @@ -824,7 +827,13 @@ export class ApolloServerBase {
// from an object containing the request and other integration specific
// options
protected async graphQLServerOptions(
integrationContextArgument?: Record<string, any>,
// We ought to be able to declare this as taking ContextFunctionParams, but
// that gets us into weird business around inheritance, since a subclass (eg
// Lambda subclassing Express) may have a different ContextFunctionParams.
// So it's the job of the subclass's function that calls this function to
// make sure that its argument properly matches the particular subclass's
// context params type.
integrationContextArgument?: any,
): Promise<GraphQLServerOptions> {
const { schema, schemaHash, documentStore } = await this._ensureStarted();

Expand Down Expand Up @@ -877,7 +886,7 @@ export class ApolloServerBase {
request: Omit<GraphQLRequest, 'query'> & {
query?: string | DocumentNode;
},
integrationContextArgument?: Record<string, any>,
integrationContextArgument?: ContextFunctionParams,
) {
// Since this function is mostly for testing, you don't need to explicitly
// start your server before calling it. (That also means you can use it with
Expand Down
5 changes: 3 additions & 2 deletions packages/apollo-server-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ export interface GraphQLService extends GatewayInterface {}

// This configuration is shared between all integrations and should include
// fields that are not specific to a single integration
export interface Config extends BaseConfig {
export interface Config<ContextFunctionParams = any>
extends BaseConfig {
modules?: GraphQLSchemaModule[];
typeDefs?: DocumentNode | Array<DocumentNode> | string | Array<string>;
parseOptions?: ParseOptions;
resolvers?: IResolvers | Array<IResolvers>;
schema?: GraphQLSchema;
context?: Context | ContextFunction;
context?: Context | ContextFunction<ContextFunctionParams>;
introspection?: boolean;
mocks?: boolean | IMocks;
mockEntireSchema?: boolean;
Expand Down
17 changes: 6 additions & 11 deletions packages/apollo-server-express/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { json, OptionsJson } from 'body-parser';
import {
GraphQLOptions,
ApolloServerBase,
ContextFunction,
Context,
Config,
runHttpQuery,
HttpQueryError,
Expand Down Expand Up @@ -41,23 +39,20 @@ export interface ExpressContext {
res: express.Response;
}

export interface ApolloServerExpressConfig extends Config {
context?: ContextFunction<ExpressContext, Context> | Context;
}

export class ApolloServer extends ApolloServerBase {
constructor(config: ApolloServerExpressConfig) {
super(config);
}
export type ApolloServerExpressConfig = Config<ExpressContext>;

export class ApolloServer<
ContextFunctionParams = ExpressContext,
> extends ApolloServerBase<ContextFunctionParams> {
// This translates the arguments from the middleware into graphQL options It
// provides typings for the integration specific behavior, ideally this would
// be propagated with a generic to the super class
async createGraphQLServerOptions(
req: express.Request,
res: express.Response,
): Promise<GraphQLOptions> {
return super.graphQLServerOptions({ req, res });
const contextParams: ExpressContext = { req, res };
return super.graphQLServerOptions(contextParams);
}

public applyMiddleware({ app, ...rest }: ServerRegistration) {
Expand Down
14 changes: 11 additions & 3 deletions packages/apollo-server-lambda/src/ApolloServer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Handler } from 'aws-lambda';
import {
ApolloServer as ApolloServerExpress,
ExpressContext,
GetMiddlewareOptions,
} from 'apollo-server-express';
import type { GraphQLOptions } from 'apollo-server-core';
Expand All @@ -16,14 +17,20 @@ export interface CreateHandlerOptions {
expressGetMiddlewareOptions?: GetMiddlewareOptions;
}

export interface LambdaContextFunctionParams {
event: ReturnType<typeof getCurrentInvoke>['event'];
context: ReturnType<typeof getCurrentInvoke>['context'];
express: ExpressContext;
}

function defaultExpressAppFromMiddleware(
middleware: express.RequestHandler,
): express.Application {
const app = express();
app.use(middleware);
return app;
}
export class ApolloServer extends ApolloServerExpress {
export class ApolloServer extends ApolloServerExpress<LambdaContextFunctionParams> {
protected override serverlessFramework(): boolean {
return true;
}
Expand Down Expand Up @@ -56,10 +63,11 @@ export class ApolloServer extends ApolloServerExpress {
res: express.Response,
): Promise<GraphQLOptions> {
const { event, context } = getCurrentInvoke();
return super.graphQLServerOptions({
const contextParams: LambdaContextFunctionParams = {
event,
context,
express: { req, res },
});
};
return super.graphQLServerOptions(contextParams);
}
}
23 changes: 21 additions & 2 deletions packages/apollo-server-lambda/src/__tests__/ApolloServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import request from 'supertest';
import express from 'express';
import { createMockServer } from './mockAPIGatewayServer';
import { Config, gql } from 'apollo-server-core';
import { ApolloServer, CreateHandlerOptions } from '../ApolloServer';
import {
ApolloServer,
CreateHandlerOptions,
LambdaContextFunctionParams,
} from '../ApolloServer';
import {
createServerInfo,
testApolloServer,
Expand Down Expand Up @@ -52,14 +56,29 @@ describe('apollo-server-lambda', () => {

const createLambda = (
createHandlerOptions: CreateHandlerOptions = {},
config: Config = { typeDefs, resolvers },
config: Config<LambdaContextFunctionParams> = { typeDefs, resolvers },
) => {
const server = new ApolloServer(config);
const handler = server.createHandler(createHandlerOptions);
return createMockServer(handler);
};

describe('context', () => {
it('context functions typecheck', async () => {
// We want to make sure that TS allows you to write the context function
// arguments. Note that the calls to createLambda that set context below
// are only good enough if we're confident that the declaration of the
// `config` argument on `createLambda` above matches the generics used in
// ApolloServer itself, so it's reasonable for us to validate against
// ApolloServer directly.
new ApolloServer({
typeDefs: 'type Query { x: Int }',
context({ event: _event, context: _context, express }) {
const { req: _req, res: _res } = express;
},
});
});

it('receives both Express and Lambda context', async () => {
const app = createLambda(
{},
Expand Down
4 changes: 2 additions & 2 deletions packages/apollo-server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import express from 'express';
import http from 'http';
import stoppable from 'stoppable';
import {
ApolloServer as ApolloServerBase,
ApolloServer as ApolloServerExpress,
CorsOptions,
ApolloServerExpressConfig,
} from 'apollo-server-express';
Expand All @@ -24,7 +24,7 @@ export interface ServerInfo {
server: http.Server;
}

export class ApolloServer extends ApolloServerBase {
export class ApolloServer extends ApolloServerExpress {
private httpServer?: stoppable.StoppableServer;
private cors?: CorsOptions | boolean;
private onHealthCheck?: (req: express.Request) => Promise<any>;
Expand Down

0 comments on commit 79ca410

Please sign in to comment.