-
Notifications
You must be signed in to change notification settings - Fork 514
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
feat: add edge support to aws-lambda presets #1557
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1557 +/- ##
==========================================
- Coverage 77.76% 74.05% -3.71%
==========================================
Files 76 74 -2
Lines 7824 7871 +47
Branches 803 756 -47
==========================================
- Hits 6084 5829 -255
- Misses 1738 2041 +303
+ Partials 2 1 -1
|
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.
Both the CDK and SST logic can be removed from this PR and included later. However for nitro-deploys
we need one or the other.
@@ -359,6 +359,7 @@ async function _build(nitro: Nitro, rollupConfig: RollupConfig) { | |||
const buildInfo = { | |||
date: new Date(), | |||
preset: nitro.options.preset, | |||
entry: nitro.options.entry.split("/").pop(), |
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.
We have no way to know which entry a preset use, maybe this should be in another 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.
Extracted in #1649
}) => | ||
/* ts */ ` | ||
import { SSTConfig } from "sst"; | ||
import { NitroApp } from "${sstDir}"; |
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.
This should probably be a relative path.
import { Nitro } from "../types"; | ||
import { writeFile } from "../utils"; | ||
|
||
export async function generateCdkApp(nitro: Nitro) { |
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.
This is more verbose and error prone than SST, harder to test and doesn't handle regular lambdas.
super.validateBuildOutput(); | ||
} | ||
|
||
protected createFunctionForRegional(): CdkFunction { |
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.
This template may need to be updated for the changes that were made to SST .
It has been updated in my test repo.
A faster way to merging might be to split SST integration and the lambda edge preset code into different PRs. (Assuming adding a dependency is what is the reservation here.)
Let the user build the deployment and add deployment guides / documentation on nitro.unjs.io / nuxt.com.
π Linked issue
Resolves #79
Related #240
Related #1075
β Type of change
π Description
Continuing on the amazing work done in #240 and #1075, this PR implements lambda edge as an option of the lambda-preset, usable with
{ target: edge }
adds a CDK construct based on @WinterYukky work, and adds a SST construct.If we want to provide something that is directly deployable (ie for
nitro-deploys
), the CDK is an interesting option, however looking closely at it, the CDK itself has a few drawbacks :Supporting CDK options is not straight forward... We could provide a Lambda-single and a Lambda-edge stack, and mirror some of these options in the config under a
cdkOptions
key. This is quite limited and I would assume most users will replace this with their own stack in real world scenario.I think the best course on action here would be to rely on the higher level constructs provided by SST, example here
https://github.com/jdevdevdev/nuxt-sst/blob/lambda-edge/stacks/NuxtSite.ts
This PR therefore adds a SST implementation that extends the
SSRSite
constructs to compare with the CDK ones.(Later we could contribute it upstream to simplify the preset)
For reference, there is also the possibility of using Amplify with the CDK.
As usual with AWS, this is poorly documented, and we should deal with this in another PR #80
(PS: I would like to give credit to @WinterYukky, @juankamilo and @jdevdevdev as co-authors of this PR)
π Checklist