-
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 aws-lambda-edge
preset with CDK
#240
Draft
WinterYukky
wants to merge
38
commits into
nitrojs:main
Choose a base branch
from
WinterYukky:feat/add-aws-lambda-edge-preset
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+600
β3
Draft
Changes from 3 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
bce1771
feat: add aws-lambda-edge preset
WinterYukky c87fde6
docs(aws-lambda-edge): add documentation
WinterYukky 69049af
docs(aws-lambda-edge): fix broken alert
WinterYukky 81759fc
feat(prest): add deployment code generater
WinterYukky 7e5deb1
fix(preset): fix empty public dir
WinterYukky 12b179a
docs(preset): update to Zero Config Provider
WinterYukky 8dd657a
docs(preset): do not use REGION env
WinterYukky a7a7af1
fix(preset): add @types/node
WinterYukky d0264ab
docs(preset): add aws to Zero-Config Providers
WinterYukky 6024e2e
docs(preset): fix message title
WinterYukky 97a89fe
feat(preset): nitro-asset to npm package
WinterYukky f174d66
docs(preset): update customization example
WinterYukky ab707c9
docs(preset): update deploy page composition
WinterYukky 79fd8e4
feat(preset): Easier to set specific region
WinterYukky ba5f8fc
docs(preset): add all option to deploy command
WinterYukky fd38fab
chore(preset): remove mkdir process
WinterYukky 9684073
docs(preset): update about bootstrap
WinterYukky 53c185a
docs(preset): add new line to bootstrap command
WinterYukky 2ac76e7
chore(preset): remove bootstrap command
WinterYukky 9b8d39a
Merge branch 'main' of https://github.com/WinterYukky/nitro into featβ¦
WinterYukky a62fa09
chore(preset): add general properties
WinterYukky c51917c
docs(preset): update to docus syntax
WinterYukky 08a7340
fix(docs): warning to alert
WinterYukky ce492b7
docs(preset): set warning type to alert
WinterYukky af349c0
docs(preset): fix warning position
WinterYukky a5245ed
Merge branch 'main' into pr/WinterYukky/240
pi0 761680c
[autofix.ci] apply automated fixes
autofix-ci[bot] 3b2d3e7
fix: append query string
pi0 35139d6
doc: fix test comment
WinterYukky 4fae105
build: bump to NODEJS_18_X
WinterYukky f93e91c
fix: remove externals property
WinterYukky cf7e69f
fix: unnormalized body
WinterYukky 7b387af
feat: support dynamic path resolve
WinterYukky 84555a2
build: use jiti
WinterYukky 52cf689
Merge branch 'main' of https://github.com/WinterYukky/nitro into featβ¦
WinterYukky 9ea6ed7
chore: apply automated fixes
autofix-ci[bot] 144d35a
test: fix to pass the new tests
WinterYukky 08a5111
doc: add AWS Lambda@Edge
WinterYukky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { defineNitroPreset } from '../preset' | ||
|
||
export const awsLambdaEdge = defineNitroPreset({ | ||
entry: '#internal/nitro/entries/aws-lambda-edge', | ||
externals: true | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import type { CloudFrontHeaders, Context, CloudFrontRequestEvent, CloudFrontResultResponse } from 'aws-lambda' | ||
import '#internal/nitro/virtual/polyfill' | ||
import { nitroApp } from '../app' | ||
|
||
export const handler = async function handler (event: CloudFrontRequestEvent, context: Context): Promise<CloudFrontResultResponse> { | ||
const request = event.Records[0].cf.request | ||
|
||
const r = await nitroApp.localCall({ | ||
event, | ||
url: request.uri, | ||
context, | ||
headers: normalizeIncomingHeaders(request.headers), | ||
method: request.method, | ||
query: request.querystring, | ||
body: request.body | ||
}) | ||
|
||
return { | ||
status: r.status.toString(), | ||
headers: normalizeOutgoingHeaders(r.headers), | ||
body: r.body.toString() | ||
} | ||
} | ||
|
||
function normalizeIncomingHeaders (headers: CloudFrontHeaders) { | ||
return Object.fromEntries(Object.entries(headers).map(([key, keyValues]) => [key, keyValues.map(kv => kv.value)])) | ||
} | ||
|
||
function normalizeOutgoingHeaders (headers: Record<string, string | string[] | undefined>): CloudFrontHeaders { | ||
return Object.fromEntries(Object.entries(headers).map(([k, v]) => [k, Array.isArray(v) ? v.map(value => ({ value })) : [{ value: v }]])) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { resolve } from 'pathe' | ||
import { describe } from 'vitest' | ||
import destr from 'destr' | ||
import type { CloudFrontHeaders, CloudFrontRequestEvent, CloudFrontResultResponse } from 'aws-lambda' | ||
import { setupTest, testNitro } from '../tests' | ||
|
||
describe('nitro:preset:aws-lambda-edge', async () => { | ||
const ctx = await setupTest('aws-lambda-edge') | ||
testNitro(ctx, async () => { | ||
const { handler } = await import(resolve(ctx.outDir, 'server/index.mjs')) | ||
return async ({ url: rawRelativeUrl, headers, method, body }) => { | ||
// creating new URL object to parse query easier | ||
const url = new URL(`https://example.com${rawRelativeUrl}`) | ||
// modify headers to CloudFrontHeaders. | ||
const reqHeaders: CloudFrontHeaders = Object.fromEntries(Object.entries(headers || {}).map(([k, v]) => [k, Array.isArray(v) ? v.map(value => ({ value })) : [{ value: v }]])) | ||
|
||
const event: CloudFrontRequestEvent = { | ||
Records: [ | ||
{ | ||
cf: { | ||
config: { | ||
distributionDomainName: 'nitro.cloudfront.net', | ||
distributionId: 'EDFDVBD6EXAMPLE', | ||
eventType: 'origin-request', | ||
requestId: '4TyzHTaYWb1GX1qTfsHhEqV6HUDd_BzoBZnwfnvQc_1oF26ClkoUSEQ==' | ||
}, | ||
request: { | ||
clientIp: '203.0.113.178', | ||
method: method || 'GET', | ||
uri: url.pathname, | ||
querystring: url.searchParams.toString(), | ||
headers: reqHeaders, | ||
body | ||
} | ||
} | ||
} | ||
] | ||
} | ||
const res: CloudFrontResultResponse = await handler(event) | ||
// responsed CloudFrontHeaders are special, so modify them for testing. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo |
||
const resHeaders = Object.fromEntries(Object.entries(res.headers).map(([key, keyValues]) => [key, keyValues.map(kv => kv.value).join(',')])) | ||
|
||
return { | ||
data: destr(res.body), | ||
status: parseInt(res.status), | ||
headers: resHeaders | ||
} | ||
} | ||
}) | ||
}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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):
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely developers can always override to customize. We can export smaller utils even for better flexibility.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.