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.

An earlier version of this PR attempted to add the context object type
(ie TContext) to the ApolloServer generics as well. But it didn't
actually connect through to the TContext on processHTTPRequest and
trying to link those up ran into some technical issues, so I decided to
leave it out.

Fixes #5480.
  • Loading branch information
glasser committed Jul 15, 2021
1 parent f1506c7 commit 3310405
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 #5481](https://github.com/apollographql/apollo-server/pull/5481)

## 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 3310405

Please sign in to comment.