-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(lambda-nodejs): esbuild detection with Yarn 2 in PnP mode #14739
Changes from 10 commits
c95f393
af8e73c
5f713ce
4bc3964
ce588e1
1cd295a
6b99302
a6347fb
d680758
7c189ec
854d926
7e5b16f
0ceab9e
d604c88
221611d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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.DockerImage; | ||||||
|
@@ -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(); | ||||||
const mustRunInDocker = !Bundling.esbuildInstallation?.version.startsWith(ESBUILD_VERSION); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering if perhaps we should just emit an error if the major version doesn't match, otherwise users will not understand why we fallback to docker. |
||||||
|
||||||
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 || mustRunInDocker; | ||||||
this.image = shouldBuildImage | ||||||
? props.dockerImage ?? cdk.DockerImage.fromBuild(path.join(__dirname, '../lib'), { | ||||||
buildArgs: { | ||||||
|
@@ -87,7 +91,12 @@ export class Bundling implements cdk.BundlingOptions { | |||||
}) | ||||||
: cdk.DockerImage.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.isLocal ? this.packageManager.runBinCommand('esbuild') : 'esbuild', | ||||||
osPlatform, | ||||||
}); | ||||||
|
||||||
this.local = { | ||||||
tryBundle(outputDir: string) { | ||||||
if (Bundling.runsLocally === false) { | ||||||
if (!Bundling.esbuildInstallation || mustRunInDocker) { | ||||||
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,31 +145,30 @@ 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}`), | ||||||
...loaders.map(([ext, name]) => `--loader:${ext}=${name}`), | ||||||
...defines.map(([key, value]) => `--define:${key}=${JSON.stringify(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; | ||||||
} | ||||||
|
||||||
/** | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { spawnSync } from 'child_process'; | ||
import { tryGetModuleVersion } from './util'; | ||
|
||
/** | ||
* An esbuild installation | ||
*/ | ||
export abstract class EsbuildInstallation { | ||
public static detect(): EsbuildInstallation | undefined { | ||
try { | ||
// Check local version first | ||
const version = tryGetModuleVersion('esbuild'); | ||
if (version) { | ||
return { | ||
isLocal: true, | ||
version, | ||
}; | ||
} | ||
|
||
// 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; | ||
} | ||
} | ||
|
||
public abstract readonly isLocal: boolean; | ||
public abstract readonly version: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(' '); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.