From 5737c336b3a2d7942196ffcad9291b4af6a23375 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Mon, 6 Dec 2021 20:58:10 +0100 Subject: [PATCH] fix(lambda-nodejs): bundling fails with a file dependency in `nodeModules` (#17851) If the dependency version is a `file:`, find its absolute path so that we can install it in the temporary bundling folder. Closes #17830 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/package-installation.ts | 4 +-- .../@aws-cdk/aws-lambda-nodejs/lib/util.ts | 35 ++++++++++++++----- .../test/package-installation.test.ts | 4 +-- .../aws-lambda-nodejs/test/util.test.ts | 16 +++++++++ 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-installation.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-installation.ts index 789246badcceb..66dfc0b6e9583 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-installation.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-installation.ts @@ -1,5 +1,5 @@ import { spawnSync } from 'child_process'; -import { tryGetModuleVersion } from './util'; +import { tryGetModuleVersionFromRequire } from './util'; /** * Package installation @@ -8,7 +8,7 @@ export abstract class PackageInstallation { public static detect(module: string): PackageInstallation | undefined { try { // Check local version first - const version = tryGetModuleVersion(module); + const version = tryGetModuleVersionFromRequire(module); if (version) { return { isLocal: true, diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts index af24d3b4ae371..bc754185cf7a6 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts @@ -73,7 +73,7 @@ export function exec(cmd: string, args: string[], options?: SpawnSyncOptions) { /** * Returns a module version by requiring its package.json file */ -export function tryGetModuleVersion(mod: string): string | undefined { +export function tryGetModuleVersionFromRequire(mod: string): string | undefined { try { // eslint-disable-next-line @typescript-eslint/no-require-imports return require(`${mod}/package.json`).version; @@ -82,6 +82,30 @@ export function tryGetModuleVersion(mod: string): string | undefined { } } +/** + * Gets module version from package.json content + */ +export function tryGetModuleVersionFromPkg(mod: string, pkgJson: { [key: string]: any }, pkgPath: string): string | undefined { + const dependencies = { + ...pkgJson.dependencies ?? {}, + ...pkgJson.devDependencies ?? {}, + ...pkgJson.peerDependencies ?? {}, + }; + + if (!dependencies[mod]) { + return undefined; + } + + // If it's a "file:" version, make it absolute + const fileMatch = dependencies[mod].match(/file:(.+)/); + if (fileMatch && !path.isAbsolute(fileMatch[1])) { + const absoluteFilePath = path.join(path.dirname(pkgPath), fileMatch[1]); + return `file:${absoluteFilePath}`; + }; + + return dependencies[mod]; +} + /** * Extract versions for a list of modules. * @@ -94,14 +118,9 @@ export function extractDependencies(pkgPath: string, modules: string[]): { [key: // Use require for cache const pkgJson = require(pkgPath); // eslint-disable-line @typescript-eslint/no-require-imports - const pkgDependencies = { - ...pkgJson.dependencies ?? {}, - ...pkgJson.devDependencies ?? {}, - ...pkgJson.peerDependencies ?? {}, - }; - for (const mod of modules) { - const version = pkgDependencies[mod] ?? tryGetModuleVersion(mod); + const version = tryGetModuleVersionFromPkg(mod, pkgJson, pkgPath) + ?? tryGetModuleVersionFromRequire(mod); if (!version) { throw new Error(`Cannot extract version for module '${mod}'. Check that it's referenced in your package.json or installed.`); } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/package-installation.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/package-installation.test.ts index e7afddc09fdbb..ce30fdb0bb148 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/package-installation.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/package-installation.test.ts @@ -13,7 +13,7 @@ test('detects local version', () => { }); test('checks global version if local detection fails', () => { - const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersion').mockReturnValue(undefined); + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersionFromRequire').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, 'tryGetModuleVersion').mockReturnValue(undefined); + const getModuleVersionMock = jest.spyOn(util, 'tryGetModuleVersionFromRequire').mockReturnValue(undefined); const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({ error: new Error('bad error'), status: 0, 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 4962ed203b31f..763a5ac499c86 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts @@ -1,4 +1,5 @@ import * as child_process from 'child_process'; +import * as fs from 'fs'; import * as path from 'path'; import { callsites, exec, extractDependencies, findUp } from '../lib/util'; @@ -119,4 +120,19 @@ describe('extractDependencies', () => { ['unknown'], )).toThrow(/Cannot extract version for module 'unknown'/); }); + + test('with file dependency', () => { + const pkgPath = path.join(__dirname, 'package-file.json'); + fs.writeFileSync(pkgPath, JSON.stringify({ + dependencies: { + 'my-module': 'file:../../core', + }, + })); + + expect(extractDependencies(pkgPath, ['my-module'])).toEqual({ + 'my-module': expect.stringMatching(/aws-cdk\/packages\/@aws-cdk\/core/), + }); + + fs.unlinkSync(pkgPath); + }); });