(aws-lambda-nodejs): "define" bundling option is not escaped

[evmoroz]

"define" bundling option appears to be insufficiently escaped

Reproduction Steps

const function = new NodejsFunction(this, 'Function', {
    entry: './app/index.ts',
    bundling: {
        define: {
            'process.env.NODE_ENV': JSON.stringify('production'),
        },
    },
})

What did you expect to happen?

Expected would be an option --define:process.env.NODE_ENV=\"production\" passed to esbuild

What actually happened?

esbuild emits a warning:

 > warning: "process.env.NODE_ENV" is defined as an identifier instead of a string (surround "production" with double quotes to get a string)

suggesting that the quotes, added by JSON.stringify were removed, I assume, by the shell executing the esbuild command

Environment

• CDK CLI Version : 1.95.1 (build ed2bbe6)
• Node.js Version: v15.12.0
• OS : Linux 5.11.9, bash 5.1.4
• Language (Version): TypeScript 4.2.3

Other

Manually escaping all double quotes with JSON.stringify('production').replace(/"/g, '\\"') is a workaround I use for now

I also expect that it would be easier to bundle the esbuild's js-api, since then there is less dealing with string escapes. (https://esbuild.github.io/getting-started/#build-scripts)

---

This is 🐛 Bug Report

[jogold]

@skyrpex you introduced the define option in #12424. Do you have similar issues?

[skyrpex]

I don't remember encountering this warning before, but maybe I overlooked. I certainly reproduce the problem and the types aren't currently correct when they are replaced by esbuild. Here's some insights:

// construct.ts
new lambdaNodejs.NodejsFunction(this, "Function", {
    entry: "function.ts",
    bundling: {
        define: {
            "process.env.VAR_1": JSON.stringify("production"),
            "process.env.VAR_2": "true",
            "process.env.VAR_3": JSON.stringify('prod"u"ction'), // This one just crashes right now
            "process.env.VAR_4": "123",
        },
    },
});

// function.ts
process.env.VAR_1;
process.env.VAR_2;
process.env.VAR_3;
process.env.VAR_4;

// function.ts (expected replacements by esbuild)
"production";
true;
'prod"u"ction';
123;

// function.ts (current incorrect replacements)
production;
true;
// process.env.VAR_3 crashes the CLI
123;

The solution seems to be to additionally JSON-stringify all of the values before sending them to esbuild via CLI. The usage of the construct is still exactly the same, but the types passed will be correct. I will happily prepare a PR.

[skyrpex]

I'm also testing replacements with multiple words like "process.env.var": "1 2 3" and it crashes with my proposed solution. Will have to think a little bit more.

Would love to use the esbuild JS API but not sure if that's feasible without refactoring quite a lot.

EDIT: OK, esbuild is not supposed to work with multiple identifier replacements. You can replace either with a single literal (either bool, string, number...) or with an identifier.

[skyrpex]

The PR is ready for review :)

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.