feat(aws-lambda-nodejs): support additional esbuild configurations#17788
Conversation
|
|
| ...this.props.banner ? [`--banner:js=${JSON.stringify(this.props.banner)}`] : [], | ||
| ...this.props.footer ? [`--footer:js=${JSON.stringify(this.props.footer)}`] : [], | ||
| ...this.props.charset ? [`--charset=${this.props.charset}`] : [], | ||
| ...this.props.esbuildArgs ? [Object.keys(this.props.esbuildArgs).map((key) => `${key}=${this.props.esbuildArgs![key]}`).join(' ')] : [], |
There was a problem hiding this comment.
| ...this.props.esbuildArgs ? [Object.keys(this.props.esbuildArgs).map((key) => `${key}=${this.props.esbuildArgs![key]}`).join(' ')] : [], | |
| ...this.props.esbuildArgs ? [Object.entries(this.props.esbuildArgs).map(([key, value]) => `${key}=${value}`).join(' ')] : [], |
+ should value be enclosed with double quotes? ${key}="${value}" to cover args where value is a path that can contain whitespaces? or should the user take care of this?
There was a problem hiding this comment.
Ah and what about args that don't have values like --keep-names?
There was a problem hiding this comment.
Good suggestions, I'll get those implemented. Thanks!
There was a problem hiding this comment.
Ah and what about args that don't have values like --keep-names?
@jogold because the type for esbuildArgs is an object we can always expect a value being assigned to each key. I think it's reasonable to assume that the user will need to pass a truthy value into the value of those boolean args.
i.g.
...
esbuildArgs: {
'--log-limit': '0',
'--resolve-extensions': '.ts,.js',
'--splitting': true,
},I'll update the type to
readonly esbuildArgs?: { [key: string]: string | boolean };I added an example of this to the README example and updated tests for clarification. LMK if you think this is acceptable.
There was a problem hiding this comment.
But esbuild doesn't accept this syntax in the CLI...
> error: Invalid build flag: "--splitting=true"
You need to update the logic to treat the Booleans differently (output only the key)
There was a problem hiding this comment.
I'm not able to reproduce this error. See my tests below:
$ esbuild src/* --bundle --outdir=output/splitting-novalue --format=esm --splitting
output/splitting-novalue/app.js 133.9kb
output/splitting-novalue/chunk-BJ7SH3MN.js 131.7kb
output/splitting-novalue/chunk-IZI55ZEC.js 447b
output/splitting-novalue/chunk-QND2VRQK.js 442b
output/splitting-novalue/GreenSpinner.js 138b
output/splitting-novalue/BlueSpinner.js 136b
output/splitting-novalue/Spinner.js 98b
$ esbuild src/* --bundle --outdir=output/splitting-withvalue-true --format=esm --splitting=true
output/splitting-withvalue-true/app.js 133.9kb
output/splitting-withvalue-true/chunk-BJ7SH3MN.js 131.7kb
output/splitting-withvalue-true/chunk-IZI55ZEC.js 447b
output/splitting-withvalue-true/chunk-QND2VRQK.js 442b
output/splitting-withvalue-true/GreenSpinner.js 138b
output/splitting-withvalue-true/BlueSpinner.js 136b
output/splitting-withvalue-true/Spinner.js 98b
$ esbuild src/* --bundle --outdir=output/splitting-withvalue-false --format=esm --splitting=false
output/splitting-withvalue-false/app.js 266.0kb
output/splitting-withvalue-false/GreenSpinner.js 132.0kb
output/splitting-withvalue-false/BlueSpinner.js 132.0kb
output/splitting-withvalue-false/Spinner.js 131.7kb
$ esbuild src/* --bundle --outdir=output/splitting-withvalue-wrapped-true --format=esm --splitting="true"
output/splitting-withvalue-wrapped-true/app.js 133.9kb
output/splitting-withvalue-wrapped-true/chunk-BJ7SH3MN.js 131.7kb
output/splitting-withvalue-wrapped-true/chunk-IZI55ZEC.js 447b
output/splitting-withvalue-wrapped-true/chunk-QND2VRQK.js 442b
output/splitting-withvalue-wrapped-true/GreenSpinner.js 138b
output/splitting-withvalue-wrapped-true/BlueSpinner.js 136b
output/splitting-withvalue-wrapped-true/Spinner.js 98b
$ esbuild src/* --bundle --outdir=output/splitting-withvalue-wrapped-false --format=esm --splitting="false"
output/splitting-withvalue-wrapped-false/app.js 266.0kb
output/splitting-withvalue-wrapped-false/GreenSpinner.js 132.0kb
output/splitting-withvalue-wrapped-false/BlueSpinner.js 132.0kb
output/splitting-withvalue-wrapped-false/Spinner.js 131.7kb
✨ Done in 1.04s.Could you share the command you're using to get that "Invalid build flag" error?
There was a problem hiding this comment.
OK it works with esbuild >= 0.14.24 published 8 days ago.
You can try with npx esbuild@0.14.23 ....
|
@nija-at any chance to review this PR? |
…esbuild-configurations
We added support for —main-fields so I replaced the example with —log-limit which we do not currently support.
…esbuild-configurations
…esbuild-configurations
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#17788) ## Summary This PR adds support for passing in any additional esbuild args that are not already exposed in the `NodejsFunction` API. With this change a user should be able to add an esbuild option such as [`--log-limit`](https://esbuild.github.io/api/#log-limit): ```ts new NodejsFunction(scope, id, { ... bundling: { esbuildArgs: { "--log-limit": "0" } } }) ``` Resolves: aws#17768 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Summary
This PR adds support for passing in any additional esbuild args that are not already exposed in the
NodejsFunctionAPI.With this change a user should be able to add an esbuild option such as
--log-limit:Resolves: #17768
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license