@aws-cdk/aws-lambda-nodejs: esbuildArgs should allow specifying alias multiple times

kirkwaiblinger commented 4 days ago

Describe the bug

Very closely related to #25385.

I want to be able to specify multiple aliases. There isn't currently a way to do that, since objects cannot have repeated keys. Given the API decision to specify possibly-repeated args in an object, I would expect to be able to indicate a repeated arg by supplying an array, like so

new NodejsFunction(this, 'test-function', {
            // ...
            bundling: {
                // ... 
                esbuildArgs: {
                    '--alias': ['@layer1=/opt/nodejs/layer1', '@layer2=/opt/nodejs/layer2']
                },
            },
        });

Regression Issue

[ ] Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

This should result in esbuild being run with --alias:@layer1=/opt/nodejs/layer1 --alias:@layer2=/opt/nodejs/layer2.

Current Behavior

(TS) Type error: Type 'string[]' is not assignable to type 'string | boolean'.

If we ignore the types and pass in an array anyway, I think at runtime this becomes --alias:"@layer1=/opt/nodejs/layer1,@layer2=/opt/nodejs/layer2", which also doesn't work.

Reproduction Steps

new NodejsFunction(this, 'test-function', {
            // ...
            bundling: {
                // ... 
                esbuildArgs: {
                    '--alias': ['@layer1=/opt/nodejs/layer1', '@layer2=/opt/nodejs/layer2']
                },
            },
        });

Possible Solution

When an array is provided as the value, it should be mapped to specify the option multiple times, i.e. --alias:@layer1=/opt/nodejs/layer1 --alias:@layer2=/opt/nodejs/layer2

Additional Information/Context

I've used essentially the same workaround as suggested in https://github.com/aws/aws-cdk/issues/25385#issuecomment-1529994229, namely

new NodejsFunction(this, 'test-function', {
            // ...
            bundling: {
                // ... 
                esbuildArgs: {
                    '--alias:@layer1': '/opt/nodejs/layer1',
                    '--alias:@layer2': '/opt/nodejs/layer2'
                },
            },
        });

While this works, it's a little anxiety-inducing, since it's a hack that depends on the implementation detail that { key: 'value' } will be used with an equal sign, to form key=value.

CDK CLI Version

2.155.0 (build 34dcc5a)

Framework Version

No response

Node.js Version

node 18

OS

macos

Language

TypeScript

Language Version

No response

Other information

No response

kirkwaiblinger commented 4 days ago

I'm thinking the crux of the change would be like so

     if (value === true || value === '') {
       args.push(key);
     } else if (reSpecifiedKeys.includes(key)) {
-      args.push(`${key}:"${value}"`);
+      const values = Array.isArray(value) ? value : [value];
+      args.push(...values.map(value => `${key}:"${value}"`));
     } else if (value) {
       args.push(`${key}="${value}"`);
     }

in https://github.com/aws/aws-cdk/blob/19ee46d7653894f0669aff3872c6c5314be0666c/packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts#L439-L454

as well as an accompanying type definitions change.

I would be happy to submit a PR for this if accepted.

khushail commented 2 days ago

Hi @kirkwaiblinger , thanks for reaching out and volunteering for contribution for the resolution of the issue.

With given code, I am able to repro the issue and can see that it gives an error by mentioning the aliases like this -

However it works by mentioning like this -

export class BundlingIssueStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new NodejsFunction(this, 'MyFunction', {
      entry: 'lib/handler.ts',
      handler: 'handler',
      bundling: {
        minify: true,
        sourceMap: true,
        esbuildArgs: {
          '--alias:@layer1': '/opt/nodejs/layer1',
          '--alias:@layer2': '/opt/nodejs/layer2'
      },
      },
    }
    )
  }
}

Please feel free to submit. a PR. I would be marking this as P3 as a workaround exists. Normal process of PR approval goes through community-review first and then core team's review. The submitted PR would get the required labels as it would go through various stages. Once you submit a PR, you could reach out to cdk.dev community for PR review. Hope that is helpful. Let me know if you need any other help.

aws / aws-cdk