From c95f393964b7840cbebf9e9dfdedd9fa9bb46a44 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 24 Feb 2021 16:33:57 +0100 Subject: [PATCH 1/6] fix(lambda-nodejs): esbuild detection with Yarn 2 in PnP mode Refactor how `esbuild` is detected and run: check for a local version first and fallback to a global installation. For local bundling, if a local version is detected, run `esbuild` using the right package manager. In Docker, always run the globally installed version. Support for PNPM could now easily be added in another PR. Closes #13179 --- .../aws-lambda-nodejs/lib/bundling.ts | 90 ++++++++++--------- .../lib/esbuild-installation.ts | 36 ++++++++ .../aws-lambda-nodejs/lib/function.ts | 5 +- .../aws-lambda-nodejs/lib/package-manager.ts | 57 ++++++++++++ .../@aws-cdk/aws-lambda-nodejs/lib/util.ts | 47 +++------- .../aws-lambda-nodejs/test/bundling.test.ts | 38 +++++--- .../test/esbuild-installation.test.ts | 64 +++++++++++++ .../test/package-manager.test.ts | 30 +++++++ .../aws-lambda-nodejs/test/util.test.ts | 70 +-------------- 9 files changed, 279 insertions(+), 158 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index a6dcddde7709d..f377870c0aae5 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -2,8 +2,10 @@ import * as os from 'os'; import * as path from 'path'; import { AssetCode, Code, Runtime } from '@aws-cdk/aws-lambda'; import * as cdk from '@aws-cdk/core'; +import { EsbuildInstallation } from './esbuild-installation'; +import { PackageManager } from './package-manager'; import { BundlingOptions } from './types'; -import { exec, extractDependencies, findUp, getEsBuildVersion, LockFile } from './util'; +import { exec, extractDependencies, findUp } from './util'; const ESBUILD_VERSION = '0'; @@ -41,11 +43,11 @@ export class Bundling implements cdk.BundlingOptions { }); } - public static clearRunsLocallyCache(): void { - this.runsLocally = undefined; + public static clearEsbuildInstallationCache(): void { + this.esbuildInstallation = undefined; } - private static runsLocally?: boolean; + private static esbuildInstallation?: EsbuildInstallation; // Core bundling options public readonly image: cdk.BundlingDockerImage; @@ -57,11 +59,13 @@ export class Bundling implements cdk.BundlingOptions { private readonly relativeEntryPath: string; private readonly relativeTsconfigPath?: string; private readonly externals: string[]; + private readonly packageManager: PackageManager; constructor(private readonly props: BundlingProps) { - Bundling.runsLocally = Bundling.runsLocally - ?? getEsBuildVersion()?.startsWith(ESBUILD_VERSION) - ?? false; + this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath); + + Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(this.packageManager); + const runsLocally = !!Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); const projectRoot = path.dirname(props.depsLockFilePath); this.relativeEntryPath = path.relative(projectRoot, path.resolve(props.entry)); @@ -76,7 +80,7 @@ export class Bundling implements cdk.BundlingOptions { ]; // Docker bundling - const shouldBuildImage = props.forceDockerBundling || !Bundling.runsLocally; + const shouldBuildImage = props.forceDockerBundling || !runsLocally; this.image = shouldBuildImage ? props.dockerImage ?? cdk.BundlingDockerImage.fromAsset(path.join(__dirname, '../lib'), { buildArgs: { @@ -87,7 +91,12 @@ export class Bundling implements cdk.BundlingOptions { }) : cdk.BundlingDockerImage.fromRegistry('dummy'); // Do not build if we don't need to - const bundlingCommand = this.createBundlingCommand(cdk.AssetStaging.BUNDLING_INPUT_DIR, cdk.AssetStaging.BUNDLING_OUTPUT_DIR); + const bundlingCommand = this.createBundlingCommand({ + inputDir: cdk.AssetStaging.BUNDLING_INPUT_DIR, + outputDir: cdk.AssetStaging.BUNDLING_OUTPUT_DIR, + esbuildRunner: 'esbuild', // esbuild is installed globally in the docker image + osPlatform: 'linux', // linux docker image + }); this.command = ['bash', '-c', bundlingCommand]; this.environment = props.environment; // Bundling sets the working directory to cdk.AssetStaging.BUNDLING_INPUT_DIR @@ -97,16 +106,21 @@ export class Bundling implements cdk.BundlingOptions { // Local bundling if (!props.forceDockerBundling) { // only if Docker is not forced const osPlatform = os.platform(); - const createLocalCommand = (outputDir: string) => this.createBundlingCommand(projectRoot, outputDir, osPlatform); + const createLocalCommand = (outputDir: string, esbuild: EsbuildInstallation) => this.createBundlingCommand({ + inputDir: projectRoot, + outputDir, + esbuildRunner: esbuild.runner, + osPlatform, + }); this.local = { tryBundle(outputDir: string) { - if (Bundling.runsLocally === false) { + if (!Bundling.esbuildInstallation || !runsLocally) { process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); return false; } - const localCommand = createLocalCommand(outputDir); + const localCommand = createLocalCommand(outputDir, Bundling.esbuildInstallation); exec( osPlatform === 'win32' ? 'cmd' : 'bash', @@ -131,19 +145,18 @@ export class Bundling implements cdk.BundlingOptions { } } - public createBundlingCommand(inputDir: string, outputDir: string, osPlatform: NodeJS.Platform = 'linux'): string { - const pathJoin = osPathJoin(osPlatform); + public createBundlingCommand(options: BundlingCommandOptions): string { + const pathJoin = osPathJoin(options.osPlatform); - const npx = osPlatform === 'win32' ? 'npx.cmd' : 'npx'; const loaders = Object.entries(this.props.loader ?? {}); const defines = Object.entries(this.props.define ?? {}); - const esbuildCommand: string = [ - npx, 'esbuild', - '--bundle', pathJoin(inputDir, this.relativeEntryPath), + const esbuildCommand: string[] = [ + options.esbuildRunner, + '--bundle', pathJoin(options.inputDir, this.relativeEntryPath), `--target=${this.props.target ?? toTarget(this.props.runtime)}`, '--platform=node', - `--outfile=${pathJoin(outputDir, 'index.js')}`, + `--outfile=${pathJoin(options.outputDir, 'index.js')}`, ...this.props.minify ? ['--minify'] : [], ...this.props.sourceMap ? ['--sourcemap'] : [], ...this.externals.map(external => `--external:${external}`), @@ -151,11 +164,11 @@ export class Bundling implements cdk.BundlingOptions { ...defines.map(([key, value]) => `--define:${key}=${value}`), ...this.props.logLevel ? [`--log-level=${this.props.logLevel}`] : [], ...this.props.keepNames ? ['--keep-names'] : [], - ...this.relativeTsconfigPath ? [`--tsconfig=${pathJoin(inputDir, this.relativeTsconfigPath)}`] : [], - ...this.props.metafile ? [`--metafile=${pathJoin(outputDir, 'index.meta.json')}`] : [], + ...this.relativeTsconfigPath ? [`--tsconfig=${pathJoin(options.inputDir, this.relativeTsconfigPath)}`] : [], + ...this.props.metafile ? [`--metafile=${pathJoin(options.outputDir, 'index.meta.json')}`] : [], ...this.props.banner ? [`--banner='${this.props.banner}'`] : [], ...this.props.footer ? [`--footer='${this.props.footer}'`] : [], - ].join(' '); + ]; let depsCommand = ''; if (this.props.nodeModules) { @@ -168,37 +181,32 @@ export class Bundling implements cdk.BundlingOptions { // Determine dependencies versions, lock file and installer const dependencies = extractDependencies(pkgPath, this.props.nodeModules); - let installer = Installer.NPM; - let lockFile = LockFile.NPM; - if (this.props.depsLockFilePath.endsWith(LockFile.YARN)) { - lockFile = LockFile.YARN; - installer = Installer.YARN; - } - - const osCommand = new OsCommand(osPlatform); + const osCommand = new OsCommand(options.osPlatform); // Create dummy package.json, copy lock file if any and then install depsCommand = chain([ - osCommand.writeJson(pathJoin(outputDir, 'package.json'), { dependencies }), - osCommand.copy(pathJoin(inputDir, lockFile), pathJoin(outputDir, lockFile)), - osCommand.changeDirectory(outputDir), - `${installer} install`, + osCommand.writeJson(pathJoin(options.outputDir, 'package.json'), { dependencies }), + osCommand.copy(pathJoin(options.inputDir, this.packageManager.lockFile), pathJoin(options.outputDir, this.packageManager.lockFile)), + osCommand.changeDirectory(options.outputDir), + this.packageManager.installCommand.join(' '), ]); } return chain([ - ...this.props.commandHooks?.beforeBundling(inputDir, outputDir) ?? [], - esbuildCommand, - ...(this.props.nodeModules && this.props.commandHooks?.beforeInstall(inputDir, outputDir)) ?? [], + ...this.props.commandHooks?.beforeBundling(options.inputDir, options.outputDir) ?? [], + esbuildCommand.join(' '), + ...(this.props.nodeModules && this.props.commandHooks?.beforeInstall(options.inputDir, options.outputDir)) ?? [], depsCommand, - ...this.props.commandHooks?.afterBundling(inputDir, outputDir) ?? [], + ...this.props.commandHooks?.afterBundling(options.inputDir, options.outputDir) ?? [], ]); } } -enum Installer { - NPM = 'npm', - YARN = 'yarn', +interface BundlingCommandOptions { + readonly inputDir: string; + readonly outputDir: string; + readonly esbuildRunner: string; + readonly osPlatform: NodeJS.Platform; } /** diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts new file mode 100644 index 0000000000000..0a1074fe575df --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts @@ -0,0 +1,36 @@ +import { spawnSync } from 'child_process'; +import { PackageManager } from './package-manager'; +import { getModuleVersion } from './util'; + +/** + * An esbuild installation + */ +export abstract class EsbuildInstallation { + public static detect(packageManager: PackageManager): EsbuildInstallation | undefined { + try { + try { + // Check local version first + const version = getModuleVersion('esbuild'); + return { + runner: packageManager.runBinCommand('esbuild'), + version, + }; + } catch (err) { + // Fallback to a global version + const esbuild = spawnSync('esbuild', ['--version']); + if (esbuild.status === 0 && !esbuild.error) { + return { + runner: 'esbuild', // the bin is globally available + version: esbuild.stdout.toString().trim(), + }; + } + return undefined; + } + } catch (err) { + return undefined; + } + } + + public abstract readonly runner: string; + public abstract readonly version: string; +} diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts index 680a6f7330356..9ad1d825e0627 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts @@ -2,8 +2,9 @@ import * as fs from 'fs'; import * as path from 'path'; import * as lambda from '@aws-cdk/aws-lambda'; import { Bundling } from './bundling'; +import { PackageManager } from './package-manager'; import { BundlingOptions } from './types'; -import { callsites, findUp, LockFile, nodeMajorVersion } from './util'; +import { callsites, findUp, nodeMajorVersion } from './util'; // keep this import separate from other imports to reduce chance for merge conflicts with v2-main // eslint-disable-next-line no-duplicate-imports, import/order @@ -95,7 +96,7 @@ export class NodejsFunction extends lambda.Function { } depsLockFilePath = path.resolve(props.depsLockFilePath); } else { - const lockFile = findUp(LockFile.YARN) ?? findUp(LockFile.NPM); + const lockFile = findUp(PackageManager.YARN.lockFile) ?? findUp(PackageManager.NPM.lockFile); if (!lockFile) { throw new Error('Cannot find a package lock file (`yarn.lock` or `package-lock.json`). Please specify it with `depsFileLockPath`.'); } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts new file mode 100644 index 0000000000000..dd5260f87838c --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-manager.ts @@ -0,0 +1,57 @@ +import * as os from 'os'; +import * as path from 'path'; + +interface PackageManagerProps { + readonly lockFile: string; + readonly installCommand: string[]; + readonly runCommand: string[]; +} + +/** + * A node package manager + */ +export class PackageManager { + public static NPM = new PackageManager({ + lockFile: 'package-lock.json', + installCommand: ['npm', 'install'], + runCommand: ['npx', '--no-install'], + }); + + public static YARN = new PackageManager({ + lockFile: 'yarn.lock', + installCommand: ['yarn', 'install'], + runCommand: ['yarn', 'run'], + }); + + public static fromLockFile(lockFilePath: string): PackageManager { + const lockFile = path.basename(lockFilePath); + + switch (lockFile) { + case PackageManager.NPM.lockFile: + return PackageManager.NPM; + case PackageManager.YARN.lockFile: + return PackageManager.YARN; + default: + return PackageManager.NPM; + } + } + + public readonly lockFile: string; + public readonly installCommand: string[]; + public readonly runCommand: string[]; + + constructor(props: PackageManagerProps) { + this.lockFile = props.lockFile; + this.installCommand = props.installCommand; + this.runCommand = props.runCommand; + } + + public runBinCommand(bin: string): string { + const [runCommand, ...runArgs] = this.runCommand; + return [ + os.platform() === 'win32' ? `${runCommand}.cmd` : runCommand, + ...runArgs, + bin, + ].join(' '); + } +} diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts index f74030f7376d1..4f99811af2d5f 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts @@ -1,6 +1,5 @@ import { spawnSync, SpawnSyncOptions } from 'child_process'; import * as fs from 'fs'; -import * as os from 'os'; import * as path from 'path'; export interface CallSite { @@ -78,6 +77,18 @@ export function exec(cmd: string, args: string[], options?: SpawnSyncOptions) { return proc; } +/** + * Returns a module version by requiring its package.json file + */ +export function getModuleVersion(mod: string): string { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + return require(`${mod}/package.json`).version; + } catch (err) { + throw new Error(`Cannot extract version for module '${mod}'`); + } +} + /** * Extract versions for a list of modules. * @@ -97,39 +108,9 @@ export function extractDependencies(pkgPath: string, modules: string[]): { [key: }; for (const mod of modules) { - try { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const version = pkgDependencies[mod] ?? require(`${mod}/package.json`).version; - dependencies[mod] = version; - } catch (err) { - throw new Error(`Cannot extract version for module '${mod}'. Check that it's referenced in your package.json or installed.`); - } + const version = pkgDependencies[mod] ?? getModuleVersion(mod); + dependencies[mod] = version; } return dependencies; } - -/** - * Returns the installed esbuild version - */ -export function getEsBuildVersion(): string | undefined { - try { - // --no-install ensures that we are checking for an installed version - // (either locally or globally) - const npx = os.platform() === 'win32' ? 'npx.cmd' : 'npx'; - const esbuild = spawnSync(npx, ['--no-install', 'esbuild', '--version']); - - if (esbuild.status !== 0 || esbuild.error) { - return undefined; - } - - return esbuild.stdout.toString().trim(); - } catch (err) { - return undefined; - } -} - -export enum LockFile { - NPM = 'package-lock.json', - YARN = 'yarn.lock' -} 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 bd69394ae757c..dabe40ad6e7ed 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -5,20 +5,26 @@ import { Code, Runtime } from '@aws-cdk/aws-lambda'; import { AssetHashType, BundlingDockerImage } from '@aws-cdk/core'; import { version as delayVersion } from 'delay/package.json'; import { Bundling } from '../lib/bundling'; +import { EsbuildInstallation } from '../lib/esbuild-installation'; import { LogLevel } from '../lib/types'; -import * as util from '../lib/util'; jest.mock('@aws-cdk/aws-lambda'); // Mock BundlingDockerImage.fromAsset() to avoid building the image -let fromAssetMock = jest.spyOn(BundlingDockerImage, 'fromAsset'); -let getEsBuildVersionMock = jest.spyOn(util, 'getEsBuildVersion'); +let fromAssetMock: jest.SpyInstance; +let detectEsbuildMock: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); jest.resetAllMocks(); - Bundling.clearRunsLocallyCache(); - getEsBuildVersionMock.mockReturnValue('0.8.8'); - fromAssetMock.mockReturnValue({ + jest.restoreAllMocks(); + Bundling.clearEsbuildInstallationCache(); + + detectEsbuildMock = jest.spyOn(EsbuildInstallation, 'detect').mockReturnValue({ + runner: 'esbuild', + version: '0.8.8', + }); + + fromAssetMock = jest.spyOn(BundlingDockerImage, 'fromAsset').mockReturnValue({ image: 'built-image', cp: () => {}, run: () => {}, @@ -53,7 +59,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', + '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 +80,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', + 'esbuild --bundle /asset-input/lib/index.ts --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk', ], }), }); @@ -94,14 +100,17 @@ 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', + 'esbuild --bundle /asset-input/lib/handler.tsx --target=node12 --platform=node --outfile=/asset-output/index.js --external:aws-sdk', ], }), }); }); test('esbuild with Windows paths', () => { - const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); + const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValueOnce('win32'); + // Mock path.basename() because it cannot extract the basename of a Windows + // path when running on Linux + jest.spyOn(path, 'basename').mockReturnValueOnce('package-lock.json'); Bundling.bundle({ entry: 'C:\\my-project\\lib\\entry.ts', @@ -139,7 +148,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', + '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,7 +190,7 @@ test('esbuild bundling with esbuild options', () => { command: [ 'bash', '-c', [ - 'npx esbuild --bundle /asset-input/lib/handler.ts', + '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"', @@ -273,7 +282,10 @@ test('Local bundling', () => { test('Incorrect esbuild version', () => { - getEsBuildVersionMock.mockReturnValueOnce('3.4.5'); + detectEsbuildMock.mockReturnValueOnce({ + runner: 'esbuild', + version: '3.4.5', + }); const bundler = new Bundling({ entry, diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts new file mode 100644 index 0000000000000..d12ffd747825b --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts @@ -0,0 +1,64 @@ +import * as child_process from 'child_process'; +import { EsbuildInstallation } from '../lib/esbuild-installation'; +import { PackageManager } from '../lib/package-manager'; +import * as util from '../lib/util'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies +const version = require('esbuild/package.json').version; + +test('NPM: returns the runner and version', () => { + const packageManager = PackageManager.NPM; + expect(EsbuildInstallation.detect(packageManager)).toEqual({ + runner: 'npx --no-install esbuild', + version, + }); +}); + +test('YARN: returns the runner and version', () => { + const packageManager = PackageManager.YARN; + expect(EsbuildInstallation.detect(packageManager)).toEqual({ + runner: 'yarn run esbuild', + version, + }); +}); + +test('checks global version if local detection fails', () => { + const getModuleVersionMock = jest.spyOn(util, 'getModuleVersion').mockImplementation(() => { throw new Error('error'); }); + const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ + status: 0, + stderr: Buffer.from('stderr'), + stdout: Buffer.from('global-version'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }); + + const packageManager = PackageManager.NPM; + expect(EsbuildInstallation.detect(packageManager)).toEqual({ + runner: 'esbuild', + version: 'global-version', + }); + + spawnSyncMock.mockRestore(); + getModuleVersionMock.mockRestore(); +}); + +test('returns undefined on error', () => { + const getModuleVersionMock = jest.spyOn(util, 'getModuleVersion').mockImplementation(() => { throw new Error('error'); }); + const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ + error: new Error('bad error'), + status: 0, + stderr: Buffer.from('stderr'), + stdout: Buffer.from('stdout'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }); + + const packageManager = PackageManager.NPM; + expect(EsbuildInstallation.detect(packageManager)).toBeUndefined(); + + spawnSyncMock.mockRestore(); + getModuleVersionMock.mockRestore(); +}); + diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts new file mode 100644 index 0000000000000..f561bce592f12 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/package-manager.test.ts @@ -0,0 +1,30 @@ +import * as os from 'os'; +import { PackageManager } from '../lib/package-manager'; + +test('from a package-lock.json', () => { + const packageManager = PackageManager.fromLockFile('/path/to/package-lock.json'); + expect(packageManager).toEqual(PackageManager.NPM); + + expect(packageManager.runBinCommand('my-bin')).toBe('npx --no-install my-bin'); +}); + +test('from a yarn.lock', () => { + const packageManager = PackageManager.fromLockFile('/path/to/yarn.lock'); + expect(packageManager).toEqual(PackageManager.YARN); + + expect(packageManager.runBinCommand('my-bin')).toBe('yarn run my-bin'); +}); + +test('defaults to NPM', () => { + const packageManager = PackageManager.fromLockFile('/path/to/pnpm-lock.yaml'); + expect(packageManager).toEqual(PackageManager.NPM); +}); + +test('Windows', () => { + const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); + + const packageManager = PackageManager.NPM; + expect(packageManager.runBinCommand('my-bin')).toEqual('npx.cmd --no-install my-bin'); + + osPlatformMock.mockRestore(); +}); diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts index df91c4433f153..4962ed203b31f 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts @@ -1,7 +1,6 @@ import * as child_process from 'child_process'; -import * as os from 'os'; import * as path from 'path'; -import { callsites, exec, extractDependencies, findUp, getEsBuildVersion } from '../lib/util'; +import { callsites, exec, extractDependencies, findUp } from '../lib/util'; beforeEach(() => { jest.clearAllMocks(); @@ -121,70 +120,3 @@ describe('extractDependencies', () => { )).toThrow(/Cannot extract version for module 'unknown'/); }); }); - -describe('getEsBuildVersion', () => { - test('returns the version', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('version'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBe('version'); - expect(spawnSyncMock).toHaveBeenCalledWith('npx', ['--no-install', 'esbuild', '--version']); - - spawnSyncMock.mockRestore(); - }); - - test('returns undefined on non zero status', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 127, // status error - stderr: Buffer.from('stderr'), - stdout: Buffer.from('stdout'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBeUndefined(); - - spawnSyncMock.mockRestore(); - }); - - test('returns undefined on error', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - error: new Error('bad error'), - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('stdout'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - expect(getEsBuildVersion()).toBeUndefined(); - - spawnSyncMock.mockRestore(); - }); - - test('Windows', () => { - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from('version'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); - - expect(getEsBuildVersion()).toBe('version'); - expect(spawnSyncMock).toHaveBeenCalledWith('npx.cmd', expect.any(Array)); - - spawnSyncMock.mockRestore(); - osPlatformMock.mockRestore(); - }); -}); From af8e73c50a5385c793b97e53fc9c58bccc007c5d Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 24 Feb 2021 17:44:37 +0100 Subject: [PATCH 2/6] isLocal --- .../aws-lambda-nodejs/lib/bundling.ts | 4 ++-- .../lib/esbuild-installation.ts | 9 ++++--- .../aws-lambda-nodejs/test/bundling.test.ts | 2 +- .../test/esbuild-installation.test.ts | 24 +++++-------------- 4 files changed, 13 insertions(+), 26 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index f377870c0aae5..181243c08d3a2 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -64,7 +64,7 @@ export class Bundling implements cdk.BundlingOptions { constructor(private readonly props: BundlingProps) { this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath); - Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(this.packageManager); + Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(); const runsLocally = !!Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); const projectRoot = path.dirname(props.depsLockFilePath); @@ -109,7 +109,7 @@ export class Bundling implements cdk.BundlingOptions { const createLocalCommand = (outputDir: string, esbuild: EsbuildInstallation) => this.createBundlingCommand({ inputDir: projectRoot, outputDir, - esbuildRunner: esbuild.runner, + esbuildRunner: esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild', osPlatform, }); diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts index 0a1074fe575df..97dd3601e16ca 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts @@ -1,18 +1,17 @@ import { spawnSync } from 'child_process'; -import { PackageManager } from './package-manager'; import { getModuleVersion } from './util'; /** * An esbuild installation */ export abstract class EsbuildInstallation { - public static detect(packageManager: PackageManager): EsbuildInstallation | undefined { + public static detect(): EsbuildInstallation | undefined { try { try { // Check local version first const version = getModuleVersion('esbuild'); return { - runner: packageManager.runBinCommand('esbuild'), + isLocal: true, version, }; } catch (err) { @@ -20,7 +19,7 @@ export abstract class EsbuildInstallation { const esbuild = spawnSync('esbuild', ['--version']); if (esbuild.status === 0 && !esbuild.error) { return { - runner: 'esbuild', // the bin is globally available + isLocal: false, version: esbuild.stdout.toString().trim(), }; } @@ -31,6 +30,6 @@ export abstract class EsbuildInstallation { } } - public abstract readonly runner: string; + public abstract readonly isLocal: boolean; public abstract readonly version: string; } 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 dabe40ad6e7ed..e3ddbc7b08e5d 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -20,7 +20,7 @@ beforeEach(() => { Bundling.clearEsbuildInstallationCache(); detectEsbuildMock = jest.spyOn(EsbuildInstallation, 'detect').mockReturnValue({ - runner: 'esbuild', + isLocal: true, version: '0.8.8', }); diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts index d12ffd747825b..27ecb33c8e2fe 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts @@ -1,23 +1,13 @@ import * as child_process from 'child_process'; import { EsbuildInstallation } from '../lib/esbuild-installation'; -import { PackageManager } from '../lib/package-manager'; import * as util from '../lib/util'; // eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies const version = require('esbuild/package.json').version; -test('NPM: returns the runner and version', () => { - const packageManager = PackageManager.NPM; - expect(EsbuildInstallation.detect(packageManager)).toEqual({ - runner: 'npx --no-install esbuild', - version, - }); -}); - -test('YARN: returns the runner and version', () => { - const packageManager = PackageManager.YARN; - expect(EsbuildInstallation.detect(packageManager)).toEqual({ - runner: 'yarn run esbuild', +test('detects local version', () => { + expect(EsbuildInstallation.detect()).toEqual({ + isLocal: true, version, }); }); @@ -33,9 +23,8 @@ test('checks global version if local detection fails', () => { signal: null, }); - const packageManager = PackageManager.NPM; - expect(EsbuildInstallation.detect(packageManager)).toEqual({ - runner: 'esbuild', + expect(EsbuildInstallation.detect()).toEqual({ + isLocal: false, version: 'global-version', }); @@ -55,8 +44,7 @@ test('returns undefined on error', () => { signal: null, }); - const packageManager = PackageManager.NPM; - expect(EsbuildInstallation.detect(packageManager)).toBeUndefined(); + expect(EsbuildInstallation.detect()).toBeUndefined(); spawnSyncMock.mockRestore(); getModuleVersionMock.mockRestore(); From 5f713ce7a25dc6b3ccb1cd9f64c0dee4178f21fe Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 24 Feb 2021 17:47:09 +0100 Subject: [PATCH 3/6] isLocal in test --- packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 e3ddbc7b08e5d..02daffd3cf2a4 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -11,8 +11,8 @@ import { LogLevel } from '../lib/types'; jest.mock('@aws-cdk/aws-lambda'); // Mock BundlingDockerImage.fromAsset() to avoid building the image -let fromAssetMock: jest.SpyInstance; -let detectEsbuildMock: jest.SpyInstance; +let fromAssetMock: jest.SpyInstance; +let detectEsbuildMock: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); jest.resetAllMocks(); @@ -283,7 +283,7 @@ test('Local bundling', () => { test('Incorrect esbuild version', () => { detectEsbuildMock.mockReturnValueOnce({ - runner: 'esbuild', + isLocal: true, version: '3.4.5', }); From 1cd295a6366e85ba0941241ab84bf8e0be39e6ed Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 1 Mar 2021 12:10:18 +0100 Subject: [PATCH 4/6] tryGetModuleVersion --- .../lib/esbuild-installation.ts | 28 +++++++++---------- .../@aws-cdk/aws-lambda-nodejs/lib/util.ts | 9 ++++-- .../test/esbuild-installation.test.ts | 4 +-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts index 97dd3601e16ca..8ef2e8dbb23d9 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/esbuild-installation.ts @@ -1,5 +1,5 @@ import { spawnSync } from 'child_process'; -import { getModuleVersion } from './util'; +import { tryGetModuleVersion } from './util'; /** * An esbuild installation @@ -7,24 +7,24 @@ import { getModuleVersion } from './util'; export abstract class EsbuildInstallation { public static detect(): EsbuildInstallation | undefined { try { - try { - // Check local version first - const version = getModuleVersion('esbuild'); + // Check local version first + const version = tryGetModuleVersion('esbuild'); + if (version) { return { isLocal: true, version, }; - } catch (err) { - // Fallback to a global version - const esbuild = spawnSync('esbuild', ['--version']); - if (esbuild.status === 0 && !esbuild.error) { - return { - isLocal: false, - version: esbuild.stdout.toString().trim(), - }; - } - return undefined; } + + // Fallback to a global version + const esbuild = spawnSync('esbuild', ['--version']); + if (esbuild.status === 0 && !esbuild.error) { + return { + isLocal: false, + version: esbuild.stdout.toString().trim(), + }; + } + return undefined; } catch (err) { return undefined; } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts index 4f99811af2d5f..07e867b05b732 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts @@ -80,12 +80,12 @@ export function exec(cmd: string, args: string[], options?: SpawnSyncOptions) { /** * Returns a module version by requiring its package.json file */ -export function getModuleVersion(mod: string): string { +export function tryGetModuleVersion(mod: string): string | undefined { try { // eslint-disable-next-line @typescript-eslint/no-require-imports return require(`${mod}/package.json`).version; } catch (err) { - throw new Error(`Cannot extract version for module '${mod}'`); + return undefined; } } @@ -108,7 +108,10 @@ export function extractDependencies(pkgPath: string, modules: string[]): { [key: }; for (const mod of modules) { - const version = pkgDependencies[mod] ?? getModuleVersion(mod); + const version = pkgDependencies[mod] ?? tryGetModuleVersion(mod); + if (!version) { + throw new Error(`Cannot extract version for module '${mod}'. Check that it's referenced in your package.json or installed.`); + } dependencies[mod] = version; } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts index 27ecb33c8e2fe..24b9b512c98bc 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/esbuild-installation.test.ts @@ -13,7 +13,7 @@ test('detects local version', () => { }); test('checks global version if local detection fails', () => { - const getModuleVersionMock = jest.spyOn(util, 'getModuleVersion').mockImplementation(() => { throw new Error('error'); }); + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersion').mockReturnValue(undefined); const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ status: 0, stderr: Buffer.from('stderr'), @@ -33,7 +33,7 @@ test('checks global version if local detection fails', () => { }); test('returns undefined on error', () => { - const getModuleVersionMock = jest.spyOn(util, 'getModuleVersion').mockImplementation(() => { throw new Error('error'); }); + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersion').mockReturnValue(undefined); const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ error: new Error('bad error'), status: 0, From d68075870a913df8e52e644148bb2ee6e3af03b9 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Tue, 18 May 2021 11:11:03 +0200 Subject: [PATCH 5/6] mustRunInDocker --- packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts | 6 +++--- packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index a00d10e9c228c..a261b4724d2e0 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -65,7 +65,7 @@ export class Bundling implements cdk.BundlingOptions { this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath); Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(); - const runsLocally = !!Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); + const mustRunInDocker = !Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); const projectRoot = path.dirname(props.depsLockFilePath); this.relativeEntryPath = path.relative(projectRoot, path.resolve(props.entry)); @@ -80,7 +80,7 @@ export class Bundling implements cdk.BundlingOptions { ]; // Docker bundling - const shouldBuildImage = props.forceDockerBundling || !runsLocally; + const shouldBuildImage = props.forceDockerBundling || mustRunInDocker; this.image = shouldBuildImage ? props.dockerImage ?? cdk.DockerImage.fromBuild(path.join(__dirname, '../lib'), { buildArgs: { @@ -115,7 +115,7 @@ export class Bundling implements cdk.BundlingOptions { this.local = { tryBundle(outputDir: string) { - if (!Bundling.esbuildInstallation || !runsLocally) { + if (!Bundling.esbuildInstallation || mustRunInDocker) { process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); return false; } 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 b6a1bc620eead..4ba234c9e0869 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -12,7 +12,7 @@ import * as util from '../lib/util'; jest.mock('@aws-cdk/aws-lambda'); // Mock DockerImage.fromAsset() to avoid building the image -let fromAssetMock: jest.SpyInstance; +let fromBuildMock: jest.SpyInstance; let detectEsbuildMock: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); @@ -25,7 +25,7 @@ beforeEach(() => { version: '0.8.8', }); - fromAssetMock = jest.spyOn(DockerImage, 'fromAsset').mockReturnValue({ + fromBuildMock = jest.spyOn(DockerImage, 'fromBuild').mockReturnValue({ image: 'built-image', cp: () => 'dest-path', run: () => {}, @@ -242,7 +242,7 @@ test('with Docker build args', () => { forceDockerBundling: true, }); - expect(fromAssetMock).toHaveBeenCalledWith(expect.stringMatching(/lib$/), expect.objectContaining({ + expect(fromBuildMock).toHaveBeenCalledWith(expect.stringMatching(/lib$/), expect.objectContaining({ buildArgs: expect.objectContaining({ HELLO: 'WORLD', }), @@ -283,7 +283,7 @@ test('Local bundling', () => { ); // Docker image is not built - expect(fromAssetMock).not.toHaveBeenCalled(); + expect(fromBuildMock).not.toHaveBeenCalled(); spawnSyncMock.mockRestore(); }); From d604c88cbd067c1de155f861a2fae36c766298a0 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 19 May 2021 11:50:57 +0200 Subject: [PATCH 6/6] throw with incorrect version --- .../aws-lambda-nodejs/lib/bundling.ts | 102 ++++++++++-------- .../aws-lambda-nodejs/test/bundling.test.ts | 7 +- 2 files changed, 60 insertions(+), 49 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts index c1e9fff1f3dc5..240114fbfa43d 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -7,7 +7,7 @@ import { PackageManager } from './package-manager'; import { BundlingOptions } from './types'; import { exec, extractDependencies, findUp } from './util'; -const ESBUILD_VERSION = '0'; +const ESBUILD_MAJOR_VERSION = '0'; /** * Bundling properties @@ -56,6 +56,7 @@ export class Bundling implements cdk.BundlingOptions { public readonly workingDirectory: string; public readonly local?: cdk.ILocalBundling; + private readonly projectRoot: string; private readonly relativeEntryPath: string; private readonly relativeTsconfigPath?: string; private readonly externals: string[]; @@ -65,13 +66,12 @@ export class Bundling implements cdk.BundlingOptions { this.packageManager = PackageManager.fromLockFile(props.depsLockFilePath); Bundling.esbuildInstallation = Bundling.esbuildInstallation ?? EsbuildInstallation.detect(); - const mustRunInDocker = !Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); - const projectRoot = path.dirname(props.depsLockFilePath); - this.relativeEntryPath = path.relative(projectRoot, path.resolve(props.entry)); + this.projectRoot = path.dirname(props.depsLockFilePath); + this.relativeEntryPath = path.relative(this.projectRoot, path.resolve(props.entry)); if (props.tsconfig) { - this.relativeTsconfigPath = path.relative(projectRoot, path.resolve(props.tsconfig)); + this.relativeTsconfigPath = path.relative(this.projectRoot, path.resolve(props.tsconfig)); } this.externals = [ @@ -80,13 +80,13 @@ export class Bundling implements cdk.BundlingOptions { ]; // Docker bundling - const shouldBuildImage = props.forceDockerBundling || mustRunInDocker; + const shouldBuildImage = props.forceDockerBundling || !Bundling.esbuildInstallation; this.image = shouldBuildImage ? props.dockerImage ?? cdk.DockerImage.fromBuild(path.join(__dirname, '../lib'), { buildArgs: { ...props.buildArgs ?? {}, - IMAGE: props.runtime.bundlingDockerImage.image, - ESBUILD_VERSION: props.esbuildVersion ?? ESBUILD_VERSION, + IMAGE: props.runtime.bundlingImage.image, + ESBUILD_VERSION: props.esbuildVersion ?? ESBUILD_MAJOR_VERSION, }, }) : cdk.DockerImage.fromRegistry('dummy'); // Do not build if we don't need to @@ -105,47 +105,11 @@ export class Bundling implements cdk.BundlingOptions { // Local bundling if (!props.forceDockerBundling) { // only if Docker is not forced - const osPlatform = os.platform(); - const createLocalCommand = (outputDir: string, esbuild: EsbuildInstallation) => this.createBundlingCommand({ - inputDir: projectRoot, - outputDir, - esbuildRunner: esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild', - osPlatform, - }); - - this.local = { - tryBundle(outputDir: string) { - if (!Bundling.esbuildInstallation || mustRunInDocker) { - process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); - return false; - } - - const localCommand = createLocalCommand(outputDir, Bundling.esbuildInstallation); - - exec( - osPlatform === 'win32' ? 'cmd' : 'bash', - [ - osPlatform === 'win32' ? '/c' : '-c', - localCommand, - ], - { - env: { ...process.env, ...props.environment ?? {} }, - stdio: [ // show output - 'ignore', // ignore stdio - process.stderr, // redirect stdout to stderr - 'inherit', // inherit stderr - ], - cwd: path.dirname(props.entry), - windowsVerbatimArguments: osPlatform === 'win32', - }); - - return true; - }, - }; + this.local = this.getLocalBundlingProvider(); } } - public createBundlingCommand(options: BundlingCommandOptions): string { + private createBundlingCommand(options: BundlingCommandOptions): string { const pathJoin = osPathJoin(options.osPlatform); const loaders = Object.entries(this.props.loader ?? {}); @@ -200,6 +164,52 @@ export class Bundling implements cdk.BundlingOptions { ...this.props.commandHooks?.afterBundling(options.inputDir, options.outputDir) ?? [], ]); } + + private getLocalBundlingProvider(): cdk.ILocalBundling { + const osPlatform = os.platform(); + const createLocalCommand = (outputDir: string, esbuild: EsbuildInstallation) => this.createBundlingCommand({ + inputDir: this.projectRoot, + outputDir, + esbuildRunner: esbuild.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild', + osPlatform, + }); + const environment = this.props.environment ?? {}; + const cwd = path.dirname(this.props.entry); + + return { + tryBundle(outputDir: string) { + if (!Bundling.esbuildInstallation) { + process.stderr.write('esbuild cannot run locally. Switching to Docker bundling.\n'); + return false; + } + + if (!Bundling.esbuildInstallation.version.startsWith(`${ESBUILD_MAJOR_VERSION}.`)) { + throw new Error(`Expected esbuild version ${ESBUILD_MAJOR_VERSION}.x but got ${Bundling.esbuildInstallation.version}`); + } + + const localCommand = createLocalCommand(outputDir, Bundling.esbuildInstallation); + + exec( + osPlatform === 'win32' ? 'cmd' : 'bash', + [ + osPlatform === 'win32' ? '/c' : '-c', + localCommand, + ], + { + env: { ...process.env, ...environment }, + stdio: [ // show output + 'ignore', // ignore stdio + process.stderr, // redirect stdout to stderr + 'inherit', // inherit stderr + ], + cwd, + windowsVerbatimArguments: osPlatform === 'win32', + }); + + return true; + }, + }; + } } interface BundlingCommandOptions { 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 c36eedd8f5c82..7d79365da5077 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -108,7 +108,7 @@ test('esbuild bundling with tsx handler', () => { }); test('esbuild with Windows paths', () => { - const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValueOnce('win32'); + const osPlatformMock = jest.spyOn(os, 'platform').mockReturnValue('win32'); // Mock path.basename() because it cannot extract the basename of a Windows // path when running on Linux jest.spyOn(path, 'basename').mockReturnValueOnce('package-lock.json'); @@ -301,8 +301,9 @@ test('Incorrect esbuild version', () => { runtime: Runtime.NODEJS_12_X, }); - const tryBundle = bundler.local?.tryBundle('/outdir', { image: Runtime.NODEJS_12_X.bundlingDockerImage }); - expect(tryBundle).toBe(false); + expect(() => bundler.local?.tryBundle('/outdir', { + image: Runtime.NODEJS_12_X.bundlingImage, + })).toThrow(/Expected esbuild version 0.x but got 3.4.5/); }); test('Custom bundling docker image', () => {