Skip to content

Commit

Permalink
fix(lambda-nodejs): parcel build cannot find target (#8838)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
jogold committed Jul 1, 2020
1 parent 99c12f5 commit ce7a015
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 16 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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`.');
}
Expand All @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts
Expand Up @@ -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, {
Expand Down
@@ -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.
Expand All @@ -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.');
}
Expand Down
17 changes: 9 additions & 8 deletions packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts
Expand Up @@ -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));
}
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
Expand Up @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand Down
13 changes: 13 additions & 0 deletions 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$/);
});

0 comments on commit ce7a015

Please sign in to comment.