Skip to content
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): parcel build cannot find target #8838

Merged
merged 3 commits into from Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts
@@ -0,0 +1,5 @@
import { findUp } from '../lib/util';

test('findUp', () => {
expect(findUp('README.md')).toBe(process.cwd());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a few more test cases here?

});