-
Notifications
You must be signed in to change notification settings - Fork 10
Added support for ALB event type #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 84028ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with the one question that may or may not need to be addressed
src/index.ts
Outdated
| | APIGatewayProxyResult | ||
| | ALBResult | ||
| ) & { | ||
| statusCode: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to assume this? I don't actually know, but the types for APIGatewayProxyStructuredResultV2 include
statusCode?: number | undefined;Are the types lying? (maybe we can submit a PR to correct them if so?)
Asserting type within the tests instead of the runtime leads to more exact runtime types without breaking the test
| httpServer.addListener('request', createMockALBServer(handler)); | ||
| httpServer.addListener( | ||
| 'request', | ||
| createMockALBServer(handler as Handler<ALBEvent, ALBResult>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I prefer this cast here instead of in the runtime, good change 👍
|
Looks like the README could be updated too? |
Implements the ALB event type. Given that V2 is very similar to ALB, I rewrote some of the normalization logic to condense parsing. Let me know your thoughts on this; after some research, it seems like there is no "official AWS supported way" of knowing exactly what event type has been passed to the Lambda. Given that, I figure we shouldn't try to parse type, and just look for the properties we need within the event object.
I also redid some of the type naming. Renamed
GatewayEventtoIncomingEventto better encapsulate ALB and lambda at edge and CloudFront functions in the future. Marked as a minor change asGatewayEventis removed.