feat(lambda-at-edge): use new aws s3 client for faster require time #583
Conversation
dphang
force-pushed
the
faster-s3-client
branch
from
609e045 to
0cb7d7f
Compare
|
This has been tested on this test app: https://d2fn5f8m5hw83u.cloudfront.net/tags/128128218 (try replacing the end number with a random number, it will load fallback until it gets the data) |
| import { performance } from "perf_hooks"; | ||
| import { ServerResponse } from "http"; | ||
| import jsonwebtoken from "jsonwebtoken"; | ||
| import { Readable } from "stream"; |
There was a problem hiding this comment.
This is just a type, should be fine to import here.
There was a problem hiding this comment.
| import { Readable } from "stream"; | |
| import type { Readable } from "stream"; |
There was a problem hiding this comment.
Oh, I thought Rollup would optimize this but I guess explicitly importing just type is good practice, too.
| const { Body } = await s3.send(new GetObjectCommand(s3Params)); | ||
|
|
||
| // Body is stream per: https://github.com/aws/aws-sdk-js-v3/issues/1096 | ||
| const getStream = await import("get-stream"); |
There was a problem hiding this comment.
I am using get-stream package: https://bundlephobia.com/[email protected] which is small
| @@ -156,8 +156,10 @@ const router = ( | |||
| export const handler = async ( | |||
| event: OriginRequestEvent | OriginResponseEvent | |||
| ): Promise<CloudFrontResultResponse | CloudFrontRequest> => { | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
do we need these @ts-ignores ?
There was a problem hiding this comment.
Hm, I saw errors when building at one point due to this being from json file, maybe this one is not needed anymore, let me try to remove it
|
Thanks for merging! |
Description
This closes #580.
@aws-sdk/client-s3(https://github.com/aws/aws-sdk-js-v3/tree/master/clients/client-s3) which is a more modularized client. It will allow us to import only the commands needed (GetObject, PutObject) instead of heavy S3 client inaws-sdk, and do it dynamically instead of in every function invocation (i.e before the handler). E.g this will only need it forgetStaticPathscases.aws-sdk/S3will load all APIs. In another PR, we could use terser with Rollup to reduce this to ~180 kB (and we can provide both minified and normal handlers).Note, I had to use
rollup-plugin-typescript2(a fork of original plugin) as I was getting these errors and could not resolve it:Though this is better in the sense that it prints more informative error messages. I also added
jsonrollup plugin as it is needed to bundle the AWS client correctly.Tests
Updated the unit tests in
origin-responsehandler test.Currently in progress of testing end-to-end.