open-constructs / aws-cdk-library

Community-Driven CDK Construct Library
Apache License 2.0
44 stars 6 forks source link

feat: Add InlineNodejsFunction to support in-project lambda handlers. #8

Closed michanto closed 4 weeks ago

michanto commented 1 month ago

Reason for this change

Inline code would be more useful if customers could write lambda handlers in their CDK projects in the language of their choice, and deploy them. This is possible in interpreted languages such as Typescript and Python. This change makes writing inline lambdas easier by utilizing the Nodejs compiler to generate javascript, rather than the user entering an uncompiled string as code.

Checklist

mrgrain commented 1 month ago

Hey congrats to your first PR!

I'm not an official reviewer, but I have the pretty strong view that we can't (or shouldn't) bundle esbuild in a jsii library. Esbuild is basically a go binary with a JavaScript interface and it does some pretty involved stuff to install the binary with the correct architecture.

For mrgrain/cdk-esbuild I went through this and couldn't get it to work. I ended up with a rather involved and silly system to automatically install esbuild when needed.

michanto commented 1 month ago

Yeah, I was pretty sure that would be the first thing pointed out. I'm not sure what the correct way to handle this is.

I know that CDK has a way of using esbuild that is not the normal, and I wasn't sure if I should go that route, this route, or put ESBUILD in devDependencies. I certainly want to be able to test this code both with and without esbuild

Should I move esbuild to devDependencies?

mrgrain commented 1 month ago

Finally while I'm not a maintainer of this package here and I'm obviously biased towards my own package, there is a question if this library does want to concern itself with problems that have already been solved by the community.

mrgrain commented 1 month ago

Yeah, I was pretty sure that would be the first thing pointed out. I'm not sure what the correct way to handle this is.

I know that CDK has a way of using esbuild that is not the normal, and I wasn't sure if I should go that route, this route, or put ESBUILD in devDependencies. I certainly want to be able to test this code both with and without esbuild

Should I move esbuild to devDependencies? I will contact you on slack to see what kinds of errors you had.

You can have a look at how upstream CDK does it. It's defaulting to docker, with a shortcut if esbuild is found locally. A devDependency would probably work to test both cases.

dbartholomae commented 1 month ago

@michanto Thanks for your contribution here! We might currently still be a bit slow in accepting PRs as we are still figuring out the best ways to work together, so please be patient with us :)

there is a question if this library does want to concern itself with problems that have already been solved by the community.

That's a good question, @mrgrain. I personally feel like one of the goals of this library could also be to make existing constructs from the community more visible and allow consumers to trust simpler processes (e.g. they might get one-time approval from their architects to use this library instead of needing approval for every individual package) - but I don't think we explicitly discussed this yet.

dbartholomae commented 1 month ago

there is a question if this library does want to concern itself with problems that have already been solved by the community.

That's a good question, @mrgrain. I personally feel like one of the goals of this library could also be to make existing constructs from the community more visible and allow consumers to trust simpler processes (e.g. they might get one-time approval from their architects to use this library instead of needing approval for every individual package) - but I don't think we explicitly discussed this yet.

I've opened an issue to discuss this fundamental question further: https://github.com/open-constructs/aws-cdk-library/issues/13

EYssel commented 1 month ago

Hi @dbartholomae

The link provided is not accessible for me.

Is it maybe a private repo?

dbartholomae commented 1 month ago

Yes, sorry, I corrected the link