chore: minify lambda function code and add source maps #1695
Conversation
Amplifiyer
added
the
run-e2e
🦋 Changeset detectedLatest commit: 151fe05 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| .split(new RegExp(`${EOL}|\n|\r`, 'g')) | ||
| .map((line) => line.replace(/\/\/.*$/, '')) // strip out inline comments because the banner is going to be flattened into a single line | ||
| .join(''); | ||
| const bannerCode = uglifyJs.minify( |
There was a problem hiding this comment.
couple of questions here:
- The banner isn't big, is it worth obscuring it?
- Is
minifyknob below using same tool under the hood? If not, should we try to align that?
I'm asking because we're introducing new dependency and I'm not sure it's worth it.
There was a problem hiding this comment.
- cc: @josefaidt as I got this as product req.
- cdk uses esbuild's minification and I couldn't find if esbuild is using any external library for minification.
There was a problem hiding this comment.
now that we're bundling the AWS SDK should we run this through esbuild with similar settings?
There was a problem hiding this comment.
We could but it seems to be an overkill.
The SSM snippet weighs around 1877 bytes.
The Bundled AWS SDK for S3 with dependencies weighs around 1390882 bytes (this is from SDK bundling prototype).
There was a problem hiding this comment.
I recommend to descope SSM snippet for now and seek esbuild based solution for it separately.
There was a problem hiding this comment.
wouldn't the aws-sdk be bundled and hence minified by the esbuild ran by aws-cdk?
There was a problem hiding this comment.
that seems fair
to @Amplifiyer 's note, it should bundle the SSM client and any dependencies it needs, but shouldn't be too heavy
There was a problem hiding this comment.
It won't minify the SSM banner that we include, that's why it was manually added here https://github.com/aws-amplify/amplify-backend/pull/1695/files#diff-e354c6c4617ce06e0e16948f4521eca5f9269445265b80ed16720372d336c886R286
There was a problem hiding this comment.
discussed offline with @Amplifiyer and decided to think about bundling/minifying the banner code separately. it'll have other implications where we'd have to think about avoiding duplication of dependency code in the resulting bundle
Problem
Issue number, if available:
Changes
Enable minifying and adding sourcemaps to underlying lambda functions bundled code.
Validation
Existing e2e passes and manually validated the minified and source maps in source code.
PS: I couldn't find a way to "inspect" the cdk assets so that I can observe the actual minification and source maps.
Checklist
run-e2elabel set.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.