fix(#18): expose aws context #19
Conversation
src/types.ts
Outdated
| export interface AwsContext extends Context { | ||
| aws: { context: any, event: AwsRequest } | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think we have to literally extend it Context. I would rather use interface declaration merging.
The syntax for this would be something like this (untested, based on memory):
declare module "@curveball/core" {
interface Context {
aws?: {
context: any,
event: any
}
}The issue with simply extending the interface, is that everything else needs to be extended (Application, middlewares, etc), or add typeguards in a bunch of different places. Neither are ideal I think.
by using declaration merging, all you have to do is install the package, and ctx.aws will be immediately available everywhere.
There was a problem hiding this comment.
I just read up on declaration merging. Interesting stuff to have in your toolbox :)
There was a problem hiding this comment.
Yea =) makes total sense too for a language where monkey patching is common
src/index.ts
Outdated
| @@ -14,7 +14,8 @@ export default function lambdaHandler(app: Application): AwsLambdaHandler { | |||
| awsRequest.isBase64Encoded ? Buffer.from(awsRequest.body, 'base64') : awsRequest.body, | |||
| ); | |||
| const response = new MemoryResponse(); | |||
| const context = new Context(request, response); | |||
| const context = new Context(request, response) as AwsContext; | |||
| context.aws = { context, event: awsRequest }; | |||
There was a problem hiding this comment.
I don't think it's needed to nest the context, I would just want to add the lambda specific objects to context.aws
moeriki
force-pushed
the
issue-18-aws-context
branch
from
a2a0ef8 to
a65a2db
Compare
moeriki
changed the title
|
I rebased to master and am testing with my project. I currently set Right now I have to wrap |
|
Everything looks great so far. There are some situations where The 2 cases I can think of are:
In each of those cases the handler will not have triggered. I feel a bit safer making it optional for that reason. If you are very certain for your specific case that it's always going to be set, you can always access the property with But taking that 'risk' of ignoring the undefined case feels a bit better if that's done by the user of the library, vs. the library itself. The user has a better idea of if this case happens to them. |
|
P.s. the linter can fix the build error with |
No description provided.