From ce7a015a973d4936e9456ff98d5f1bef58642730 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 1 Jul 2020 18:25:36 +0200 Subject: [PATCH] fix(lambda-nodejs): parcel build cannot find target (#8838) The `PackageJsonManager` was always targeting the `package.json` of `@aws-cdk/aws-lambda-nodejs` and not the one closest to the entry file. This was not detected in the tests inside the repo because both files are the same. Closes #8837 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-lambda-nodejs/lib/bundling.ts | 6 +++--- .../@aws-cdk/aws-lambda-nodejs/lib/function.ts | 5 +++-- .../lib/package-json-manager.ts | 6 +++--- packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts | 17 +++++++++-------- .../aws-lambda-nodejs/test/bundling.test.ts | 12 ++++++++++++ .../aws-lambda-nodejs/test/util.test.ts | 13 +++++++++++++ 6 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda-nodejs/test/util.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 274e043bf11a3..063d1b2cbff62 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts @@ -3,7 +3,7 @@ import * as cdk from '@aws-cdk/core'; import * as fs from 'fs'; import * as path from 'path'; import { PackageJsonManager } from './package-json-manager'; -import { findClosestPathContaining } from './util'; +import { findUp } from './util'; /** * Base options for Parcel bundling @@ -97,7 +97,7 @@ export class Bundling { */ public static parcel(options: ParcelOptions): lambda.AssetCode { // Find project root - const projectRoot = options.projectRoot ?? findClosestPathContaining(`.git${path.sep}`); + const projectRoot = options.projectRoot ?? findUp(`.git${path.sep}`); if (!projectRoot) { throw new Error('Cannot find project root. Please specify it with `projectRoot`.'); } @@ -110,7 +110,7 @@ export class Bundling { }, }); - const packageJsonManager = new PackageJsonManager(); + const packageJsonManager = new PackageJsonManager(path.dirname(options.entry)); // Collect external and install modules let includeNodeModules: { [key: string]: boolean } | undefined; diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts index 70cfb8f19b130..1138807443fec 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts @@ -54,8 +54,9 @@ export class NodejsFunction extends lambda.Function { : lambda.Runtime.NODEJS_10_X; const runtime = props.runtime ?? defaultRunTime; - // We need to restore the package.json after bundling - const packageJsonManager = new PackageJsonManager(); + // Look for the closest package.json starting in the directory of the entry + // file. We need to restore it after bundling. + const packageJsonManager = new PackageJsonManager(path.dirname(entry)); try { super(scope, id, { diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-json-manager.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-json-manager.ts index c43c4b0963351..07e7354d0ebe5 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/package-json-manager.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/package-json-manager.ts @@ -1,6 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; -import { findClosestPathContaining } from './util'; +import { findUp } from './util'; /** * A package.json manager to act on the closest package.json file. @@ -13,8 +13,8 @@ export class PackageJsonManager { private readonly pkg: Buffer; private readonly pkgJson: PackageJson; - constructor() { - const pkgPath = findClosestPathContaining('package.json'); + constructor(directory: string) { + const pkgPath = findUp('package.json', directory); if (!pkgPath) { throw new Error('Cannot find a `package.json` in this project.'); } diff --git a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts index 92d61e7f76e1a..6b3f7f8173a02 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts @@ -51,14 +51,15 @@ export function nodeMajorVersion(): number { } /** - * Finds the closest path containg a path + * Find a file by walking up parent directories */ -export function findClosestPathContaining(p: string): string | undefined { - for (const nodeModulesPath of module.paths) { - if (fs.existsSync(path.join(path.dirname(nodeModulesPath), p))) { - return path.dirname(nodeModulesPath); - } +export function findUp(name: string, directory: string = process.cwd()): string | undefined { + const { root } = path.parse(directory); + if (directory === root && !fs.existsSync(path.join(directory, name))) { + return undefined; } - - return undefined; + if (fs.existsSync(path.join(directory, name))) { + return directory; + } + return findUp(name, path.dirname(directory)); } 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 e7129cf5da0c8..aaa1553943f0a 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -3,12 +3,21 @@ import { Code, Runtime } from '@aws-cdk/aws-lambda'; import { AssetHashType } from '@aws-cdk/core'; import { version as delayVersion } from 'delay/package.json'; import * as fs from 'fs'; +import * as path from 'path'; import { Bundling } from '../lib/bundling'; +import * as util from '../lib/util'; jest.mock('@aws-cdk/aws-lambda'); const writeFileSyncMock = jest.spyOn(fs, 'writeFileSync').mockReturnValue(); const existsSyncOriginal = fs.existsSync; const existsSyncMock = jest.spyOn(fs, 'existsSync'); +const originalFindUp = util.findUp; +const findUpMock = jest.spyOn(util, 'findUp').mockImplementation((name: string, directory) => { + if (name === 'package.json') { + return path.join(__dirname, '..'); + } + return originalFindUp(name, directory); +}); beforeEach(() => { jest.clearAllMocks(); @@ -59,6 +68,9 @@ test('Parcel bundling', () => { }, }, })); + + // Searches for the package.json starting in the directory of the entry file + expect(findUpMock).toHaveBeenCalledWith('package.json', '/project/folder'); }); test('Parcel with Windows paths', () => { diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts new file mode 100644 index 0000000000000..026f2bcb519e3 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts @@ -0,0 +1,13 @@ +import * as path from 'path'; +import { findUp } from '../lib/util'; + +test('findUp', () => { + // Starting at process.cwd() + expect(findUp('README.md')).toMatch(/aws-lambda-nodejs$/); + + // Non existing file + expect(findUp('non-existing-file.unknown')).toBe(undefined); + + // Starting at a specific path + expect(findUp('util.test.ts', path.join(__dirname, 'integ-handlers'))).toMatch(/aws-lambda-nodejs\/test$/); +});