From f06ef5b788f1193a484d767d17b416dbb1a67abd Mon Sep 17 00:00:00 2001 From: Gerald McAlister Date: Sun, 28 Feb 2021 02:35:56 +0000 Subject: [PATCH 1/2] fix(aws-lambda-nodejs): Paths with Spaces Break ESBuild [Problem] Paths with spaces break ESBuild on Windows. [Solution] Add double quotes around the input paths for the ESBuild command. [Testing] Updated unit tests and confirmed in my own package this fix works. --- packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts | 4 ++-- .../@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index 5051fc9012ada..ed21f45a46075 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -140,10 +140,10 @@ export class Bundling implements cdk.BundlingOptions { const esbuildCommand: string = [ npx, 'esbuild', - '--bundle', pathJoin(inputDir, this.relativeEntryPath).replace(/(\s+)/g, '\\$1'), + '--bundle', `"${pathJoin(inputDir, this.relativeEntryPath).replace(/(\s+)/g, '\\$1')}"`, `--target=${this.props.target ?? toTarget(this.props.runtime)}`, '--platform=node', - `--outfile=${pathJoin(outputDir, 'index.js')}`, + `--outfile="${pathJoin(outputDir, 'index.js')}"`, ...this.props.minify ? ['--minify'] : [], ...this.props.sourceMap ? ['--sourcemap'] : [], ...this.externals.map(external => `--external:${external}`), diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts index 2f96c10ff78ed..022dee284a78a 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -53,7 +53,7 @@ test('esbuild bundling in Docker', () => { }, command: [ 'bash', '-c', - 'npx esbuild --bundle /asset-input/lib/handler.ts --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk --loader:.png=dataurl', + 'npx esbuild --bundle "/asset-input/lib/handler.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk --loader:.png=dataurl', ], workingDirectory: '/', }), @@ -74,7 +74,7 @@ test('esbuild bundling with handler named index.ts', () => { bundling: expect.objectContaining({ command: [ 'bash', '-c', - 'npx esbuild --bundle /asset-input/lib/index.ts --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk', + 'npx esbuild --bundle "/asset-input/lib/index.ts" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', ], }), }); @@ -94,7 +94,7 @@ test('esbuild bundling with tsx handler', () => { bundling: expect.objectContaining({ command: [ 'bash', '-c', - 'npx esbuild --bundle /asset-input/lib/handler.tsx --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk', + 'npx esbuild --bundle "/asset-input/lib/handler.tsx" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:aws-sdk', ], }), }); @@ -139,7 +139,7 @@ test('esbuild bundling with externals and dependencies', () => { command: [ 'bash', '-c', [ - 'npx esbuild --bundle /asset-input/test/bundling.test.js --target=node12 --platform=node --outfile=/asset-output/index.js --external:abc --external:delay', + 'npx esbuild --bundle "/asset-input/test/bundling.test.js" --target=node12 --platform=node --outfile="/asset-output/index.js" --external:abc --external:delay', `echo \'{\"dependencies\":{\"delay\":\"${delayVersion}\"}}\' > /asset-output/package.json`, 'cp /asset-input/package-lock.json /asset-output/package-lock.json', 'cd /asset-output', @@ -181,8 +181,8 @@ test('esbuild bundling with esbuild options', () => { command: [ 'bash', '-c', [ - 'npx esbuild --bundle /asset-input/lib/handler.ts', - '--target=es2020 --platform=node --outfile=/asset-output/index.js', + 'npx esbuild --bundle "/asset-input/lib/handler.ts"', + '--target=es2020 --platform=node --outfile="/asset-output/index.js"', '--minify --sourcemap --external:aws-sdk --loader:.png=dataurl', '--define:DEBUG=true --define:process.env.KEY="VALUE"', '--log-level=silent --keep-names --tsconfig=/asset-input/lib/custom-tsconfig.ts', From 12d9d3071edfb49271c5fce6ca80a5541a1fcddc Mon Sep 17 00:00:00 2001 From: Gerald McAlister Date: Sat, 27 Feb 2021 19:04:14 -0800 Subject: [PATCH 2/2] fix(aws-lambda-nodejs): Paths with Spaces Break ESBuild - 2/X [Problem] Need to revert the previous change that escaped spaces for Windows builds, since this did not actually fix the issue. [Solution] Removed previous code changes. [Testing] Updated unit tests and manually tested on another project. --- .../aws-lambda-nodejs/lib/bundling.ts | 2 +- .../aws-lambda-nodejs/test/bundling.test.ts | 20 ------------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index ed21f45a46075..536ca1ea7646a 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -140,7 +140,7 @@ export class Bundling implements cdk.BundlingOptions { const esbuildCommand: string = [ npx, 'esbuild', - '--bundle', `"${pathJoin(inputDir, this.relativeEntryPath).replace(/(\s+)/g, '\\$1')}"`, + '--bundle', `"${pathJoin(inputDir, this.relativeEntryPath)}"`, `--target=${this.props.target ?? toTarget(this.props.runtime)}`, '--platform=node', `--outfile="${pathJoin(outputDir, 'index.js')}"`, diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts index 022dee284a78a..382baafee54a0 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -334,23 +334,3 @@ test('with command hooks', () => { }), }); }); - -test('escapes spaces in path', () => { - Bundling.bundle({ - entry: '/project/lib/my cool lambda/handler.ts', - depsLockFilePath, - runtime: Runtime.NODEJS_12_X, - forceDockerBundling: true, - }); - - // Correctly bundles with esbuild - expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(depsLockFilePath), { - assetHashType: AssetHashType.OUTPUT, - bundling: expect.objectContaining({ - command: [ - 'bash', '-c', - expect.stringContaining('lib/my\\ cool\\ lambda/handler.ts'), - ], - }), - }); -});