apollo-server-lambda: Fix callback requirement for handler

[joshbalfour]

This is for #5592.

I copied the definition of Handler from @types/aws-lambda and made callback optional, which fixes the issue.

Hope this approach is alright, please let me know your thoughts.

Thanks!

[glasser]

Hmm. I think you're right that we shouldn't claim we're implementing the @types/aws-lambda Handler since as of AS3 we don't pay attention to the callback. But if we're going to define our own type, shouldn't we just drop the callback argument from the type entirely? I think that a passed callback gets ignored (if you dig in to @vendia/serverless-express, we are using the default resolutionMode of PROMISE, so the callback is ignored).

[joshbalfour]

Ahh, makes sense 👍

[glasser]

I think this doesn't build (because it doesn't match the type returned from @vendia/serverless-lambda, which is the exact type from aws-lambda we're trying to move away from). I'm not quite sure what to do here, other than perhaps @ts-ignore? Or a patch to https://github.com/vendia/serverless-express to make the return type depend on what is passed as resolutionMethod (which is actually something TypeScript supports, via multiple declarations for a function).

[glasser]

(should have clicked this button yesterday)

[glasser]

(Another option would be to send DefinitelyTyped a PR to make their type more flexible or to provide another non-callback handler type!)

[shellscape]

@glasser just ran into this today. what needs to be done to get this change merged? I'll pick up the effort tomorrow to make it happen.

[glasser]

Unless something has changed, a PR to serverless-express to use type overloads to make its return type sensitive to how it is called would be best.

[shellscape]

I'm a bit confused. I don't see any mention of serverless-express here

apollo-server/packages/apollo-server-lambda/src/ApolloServer.ts

Line 1 in 388e650

import type { Handler } from 'aws-lambda';

wrt to the Handler type. Are you saying you'd prefer that this package didn't maintain that type, and instead ask serverless-express to export a flexible Handler type?

[glasser]

So, as you can see in the CI, this PR does not compile. That is because the serverlessExpress function we depend on (the default export of this file: https://github.com/vendia/serverless-express/blob/mainline/src/configure.d.ts) says it returns one of aws-lambda's Handlers. That's a function that expects three non-optional arguments. So assigning that to a type that allows you to call it with only two arguments can't compile.

Now, the serverlessExpress function is flexible: it takes a resolutionMode parameter which defaults to 'PROMISE' but can also be other values like 'CALLBACK'. One could change configure.d.ts in that package so that when resolutionMode is not passed (or is passed as PROMISE or CONTEXT) then the callback argument is not required. Then this change here would build. It would be a nice improvement! (There's some issues here around how technically you can specify resolutionMode in a second call to a deprecated function later in the file but maybe folks using that deprecated function could get worse types.)

[jorenvh1]

Any updates on this issue?

[glasser]

Feel free to take over with your own PR — as described above, the PR as written does not build.

Note that in the upcoming Apollo Server 4, the Lambda integration will be less tightly integrated into core and there can instead be multiple community approaches to Lambda integration — perhaps a direct implementation of the Lambda data structures, perhaps another using @vendia/serverless-express like we do now, perhaps another using Middy...

[glasser]

Apollo Server 4 replaces a hard-coded set of web framework integrations with a simple stable API for building integrations. As part of this, the core project no longer directly supports a Lambda integration. Check out the @as-integrations/aws-lambda package; this new package is maintained by Lambda users and implements support for the API Gateway v1 and v2 protocols. If you need to work with different AWS projects, you may be able to add that support to that package, or take another approach such as combining the @vendia/serverless-express package with Apollo Server 4's built-in expressMiddleware (this is the same approach that apollo-server-lambda takes in AS3). Sorry for the delay!