dbartholomae / lambda-middleware

A collection of middleware for AWS lambda functions.
https://dbartholomae.github.io/lambda-middleware/
MIT License
151 stars 18 forks source link

Updated deserializeBody middleware to also work on APIGatewayProxyEventV2 event types #71

Closed abmohan closed 1 year ago

abmohan commented 2 years ago

Closes #70

Checklist:

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

abmohan commented 2 years ago

@dbartholomae took a stab at this. It's my first time making a PR to the repo and using the tooling (e.g., rush).

How does it look? Apologies if it's not the format and process you were looking for; I'm happy to revise as necessary.

Also, do we have a discord server available?

dbartholomae commented 2 years ago

Thanks! I'll take a closer look on the weekend :)

dbartholomae commented 2 years ago

Thanks for taking a stab at this! Unfortunately, I don't think it's this simple:

To be able to use the middleware with an APIGatewayProxyEventV2, the src/JsonDeserializer.ts would need to accept events that extend APIGatewayProxyEventV2, not only APIGatewayProxyEvent. I don't think changing only the internal types would make a difference.

Unfortunately this would mean that also the handler return type and the handler event type would need to be adjusted.

This becomes a bit complicated, because both types need to exist next to each other: If the incoming event is an APIGatewayProxyEvent, then the handler needs to get APIGatewayProxyObjectEvent<APIGatewayProxyEvent>, but if the incoming event is an APIGatewayProxyEventV2, then the handler needs to get APIGatewayProxyObjectEvent<APIGatewayProxyEventV2>.

I'm not sure how to best solve this off the back of my head. Ideally, we could reduce the required type to just the parts that are actually needed for the middleware, something like

{
    body?: string | null;
    headers?: Record<string, string>;
    isBase64Encoded: boolean;
  }

I haven't checked yet whether this would work, though.

abmohan commented 2 years ago

Cool makes sense. I'll take a stab at this later this week!

github-actions[bot] commented 2 years ago

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.