feat: add aws-lambda-edge preset with CDK
#240
Conversation
|
I support this PR :) (@WinterYukky You might consider a review request?) |
|
@mirumirumi Thanks your comment! Also, thank you @danielroe and @pi0 for reviewing🥰 |
docs/deploy/providers/aws.md
Outdated
| The following code is an example of deploying a Nuxt3 project to CloudFront and Lambda@Edge with [AWS CDK](https://github.com/aws/aws-cdk). Using this stack, paths under `_nuxt/` (static assets) will get their data from the S3 origin, and all other paths will be resolved by Lambda@Edge. | ||
|
|
||
| ```ts | ||
| import { spawnSync } from "child_process"; |
There was a problem hiding this comment.
I think we can auto generate this script to the .output. avoiding to hardcode things like public path to the example.
There was a problem hiding this comment.
You mean the app build part? I'd like that. Maybe the output could be an object containing the key locations and configs that can be used for cloud specific deployments (for example configuring the S3 bucket and cloudfront caching):
{
"serverHandler": ".output/server",
"assets": ".output/assets"
"public": ".output/public"
}There was a problem hiding this comment.
Thanks for the review.
This script is not a simple script, it is IaC, so it may be difficult to put it in the .output from a DX perspective.
I think a developer wants more freedom to customize it.
Is the key here that the developer needs to be aware of the public and server paths?
There was a problem hiding this comment.
Surely developers can always override to customize. We can export smaller utils even for better flexibility.
Is the key here that the developer needs to be aware of the public and server paths?
Yes. Such things shouldn't be hardcoded into the repository code or docs but auto-generated even considering customization needs.
There was a problem hiding this comment.
I've been considering various use cases and interfaces for the past week, but I think I've been overthinking things a bit.
As @chris-visser mentioned, it might be better to just include the serverDir and publicDir in nitro.json.
What do you think of that idea, @pi0?
There was a problem hiding this comment.
On the other hand. These settings are easily extracted from the nuxt.config.ts since they are either Nuxt's defaults or set in that config. So maybe its not even needed.
There was a problem hiding this comment.
Yes I can agree your thinking @chris-visser. However to find configuration like nuxt.config.ts or nitro.config.ts and more... is a bit difficult for CDK apps.
Also maybe understood @pi0's thinking. His goal would be a Zero-Config Providers. Certainly that is one of the important features so I will try implemantation for auto generate this script to the .output.
|
Hi @WinterYukky Thanks for your works on this pull request and sorry review took long. I will have to try the deployment and probably pushing some improvements to automate as much as possible for sdk use. (Hense self assigned). |
|
Hi @pi0. |
|
Any news about this pr? This is quite important for AWS use cases, as stated here: #1106 |
|
@danielroe @pi0 I've noticed that there's 2 PR open for aws-lamba-edge Which one should get merged ? |
the other one is newer, although it seems very similar to this one, while this one also has CDK and github action setup, which is very interesting! |
|
Users deploying through IaC (SST/Terraform/Pulumi/Cloudformation) other than CDK and would find Cloudfront wrapper useful. It would require decoupling it from the CDK deployment. Possible solutions could be to:
|
Hebilicious
changed the title
aws-lambda-edge presetaws-lambda-edge preset with CDK
There was a problem hiding this comment.
Amazing work @WinterYukky
The documentation added here is incredible, and will be extremely useful for cdk support #1387
@pi0 I believe a good course of action here would be to merge
#1075 over this PR, and use this one as the base for CDK support both in lambda and lambda-edge
| headers: normalizeIncomingHeaders(request.headers), | ||
| method: request.method, | ||
| query: request.querystring, | ||
| body: request.body, |
There was a problem hiding this comment.
body should be normalized
There was a problem hiding this comment.
test/presets/aws-lambda-edge.test.ts
Outdated
| ] | ||
| } | ||
| const res: CloudFrontResultResponse = await handler(event) | ||
| // responsed CloudFrontHeaders are special, so modify them for testing. |
src/presets/aws-lambda-edge.ts
Outdated
|
|
||
| export const awsLambdaEdge = defineNitroPreset({ | ||
| entry: "#internal/nitro/entries/aws-lambda-edge", | ||
| externals: true, |
There was a problem hiding this comment.
this can't be a boolean
There was a problem hiding this comment.
It was unnecessary property so remove it.
src/presets/aws-lambda-edge.ts
Outdated
| await writeFile( | ||
| resolve(cdkDir, "cdk.json"), | ||
| JSON.stringify({ | ||
| app: "npx ts-node --prefer-ts-exts bin/nitro-lambda-edge.ts", |
There was a problem hiding this comment.
could use jiti instead of ts-node here
There was a problem hiding this comment.
I didn't know jiti before I had recieve this comment! Thank you!!
src/presets/aws-lambda-edge.ts
Outdated
| this, | ||
| "EdgeFunction", | ||
| { | ||
| runtime: lambda.Runtime.NODEJS_16_X, |
|
Thanks for your reviewing @Hebilicious. |
…/add-aws-lambda-edge-preset
|
@Hebilicious Thanks your reviewing!! |
🔗 Linked issue
❓ Type of change
📚 Description
Add AWS Lambda@Edge to the preset.
The current
aws-lambdapreset is not compatible with the Lambda@Edge format and requires users to create their own wrapper for Lambda@Edge. This PR is needed to resolve this issue.It would also be very powerful if Lambda@Edge were available in Nitro. It will be quite important for projects that require SSR (e.g Nuxt3) as static assets (
.output/public) can be retrieved from AWS S3 and the rest (.output/server) can be resolved by Lambda@Edge.Resolves #79
📝 Checklist