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.

Sign up for GitHub

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

Jump to bottom

fix(lambda-nodejs): cannot bundle when entry file is named index.ts #9724

Merged
merged 3 commits into from
Aug 17, 2020
Merged

fix(lambda-nodejs): cannot bundle when entry file is named index.ts #9724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77ec964
fix(lambda-nodejs): cannot bundle when entry file is named index.ts
jogold Aug 14, 2020
e78fe67
Merge branch 'master' into lambda-nodejs-index-fail
Aug 17, 2020
8720697
Merge branch 'master' into lambda-nodejs-index-fail
mergify[bot] Aug 17, 2020
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
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/lib/bundling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export class Bundling {
// Always rename dist file to index.js because Lambda doesn't support filenames
// with multiple dots and we can end up with multiple dots when using automatic
// entry lookup
`mv ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/${distFile} ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/index.js`,
distFile !== 'index.js' ? `mv ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/${distFile} ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/index.js` : '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be less error prone to just have parcel output as index.js? parcel-bundler/parcel#884

Doesn't look like using the entry name has any value and we could remove the parsing on line 155.

Suggested change
distFile !== 'index.js' ? `mv ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/${distFile} ${cdk.AssetStaging.BUNDLING_OUTPUT_DIR}/index.js` : '',
'--out-file index.js'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately --out-file is a Parcel v1 option, not v2. In v2 you need to configure it in the package.json but I want to avoid this to allow #9632

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. Parcel seems like a pile.

]);

let installer = Installer.NPM;
Expand Down
19 changes: 19 additions & 0 deletions packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@ test('Parcel bundling', () => {
expect(findUpMock).toHaveBeenCalledWith('package.json', '/project/folder');
});

test('Parcel bundling with handler named index.ts', () => {
Bundling.parcel({
entry: '/project/folder/index.ts',
runtime: Runtime.NODEJS_12_X,
projectRoot: '/project',
});

// Correctly bundles with parcel
expect(Code.fromAsset).toHaveBeenCalledWith('/project', {
assetHashType: AssetHashType.BUNDLE,
bundling: expect.objectContaining({
command: [
'bash', '-c',
'$(node -p "require.resolve(\'parcel\')") build /asset-input/folder/index.ts --target cdk-lambda --dist-dir /asset-output --no-autoinstall --no-scope-hoist',
],
}),
});
});

test('Parcel with Windows paths', () => {
Bundling.parcel({
entry: 'C:\\my-project\\lib\\entry.ts',
Expand Down